* [PATCH v2 1/4] Define ePMP mseccfg
[not found] <20200811002325.46056-1-weiying_hou@outlook.com>
@ 2020-08-11 0:23 ` Hou Weiying
2020-08-11 0:23 ` Hou Weiying
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: sagark, kbastian, Hongzheng-Li, Alistair.Francis, Myriad-Dreamin, palmer
Currently using 0x390 and 0x391 for x-epmp (experimental). This may change in the future spec.
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/cpu_bits.h | 3 +++
target/riscv/gdbstub.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 8117e8b5a7..9c35179983 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -229,6 +229,9 @@
#define CSR_MTINST 0x34a
#define CSR_MTVAL2 0x34b
+/* Enhanced PMP */
+#define CSR_MSECCFG 0x390
+#define CSR_MSECCFGH 0x391
/* Physical Memory Protection */
#define CSR_PMPCFG0 0x3a0
#define CSR_PMPCFG1 0x3a1
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index eba12a86f2..de5551604a 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -132,6 +132,8 @@ static int csr_register_map[] = {
CSR_MIP,
CSR_MTINST,
CSR_MTVAL2,
+ CSR_MSECCFG,
+ CSR_MSECCFGH,
CSR_PMPCFG0,
CSR_PMPCFG1,
CSR_PMPCFG2,
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 1/4] Define ePMP mseccfg
@ 2020-08-11 0:23 ` Hou Weiying
0 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: palmer, Alistair.Francis, sagark, kbastian, Hongzheng-Li, Myriad-Dreamin
Currently using 0x390 and 0x391 for x-epmp (experimental). This may change in the future spec.
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/cpu_bits.h | 3 +++
target/riscv/gdbstub.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 8117e8b5a7..9c35179983 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -229,6 +229,9 @@
#define CSR_MTINST 0x34a
#define CSR_MTVAL2 0x34b
+/* Enhanced PMP */
+#define CSR_MSECCFG 0x390
+#define CSR_MSECCFGH 0x391
/* Physical Memory Protection */
#define CSR_PMPCFG0 0x3a0
#define CSR_PMPCFG1 0x3a1
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index eba12a86f2..de5551604a 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -132,6 +132,8 @@ static int csr_register_map[] = {
CSR_MIP,
CSR_MTINST,
CSR_MTVAL2,
+ CSR_MSECCFG,
+ CSR_MSECCFGH,
CSR_PMPCFG0,
CSR_PMPCFG1,
CSR_PMPCFG2,
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/4] Implementation of enhanced PMP(ePMP) support
[not found] <20200811002325.46056-1-weiying_hou@outlook.com>
@ 2020-08-11 0:23 ` Hou Weiying
2020-08-11 0:23 ` Hou Weiying
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: sagark, kbastian, Hongzheng-Li, Alistair.Francis, Myriad-Dreamin, palmer
The ePMP can be found in:
https://docs.google.com/document/d/1Mh_aiHYxemL0umN3GTTw8vsbmzHZ_nxZXgjgOUzbvc8/edit#heading=h.9wsr1lnxtwe2
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/pmp.c | 134 ++++++++++++++++++++++++++++++++++----
target/riscv/pmp.h | 12 ++++
target/riscv/trace-events | 4 ++
3 files changed, 138 insertions(+), 12 deletions(-)
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 2a2b9f5363..b1fa703aff 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -34,6 +34,26 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index);
+static char mode_to_char(int mode)
+{
+ char ret = 0;
+ switch (mode) {
+ case PRV_U:
+ ret = 'u';
+ break;
+ case PRV_S:
+ ret = 's';
+ break;
+ case PRV_H:
+ ret = 'h';
+ break;
+ case PRV_M:
+ ret = 'm';
+ break;
+ }
+ return ret;
+}
+
/*
* Accessor method to extract address matching type 'a field' from cfg reg
*/
@@ -99,7 +119,28 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t pmp_index)
static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
{
if (pmp_index < MAX_RISCV_PMPS) {
- if (!pmp_is_locked(env, pmp_index)) {
+ /*
+ * mseccfg.RLB is set
+ */
+ if (MSECCFG_RLB_ISSET(env) ||
+ /*
+ * mseccfg.MML is set
+ */
+ (MSECCFG_MML_ISSET(env) &&
+ /*
+ * m model and not adding X bit
+ */
+ (((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) ||
+ /*
+ * shared region and not adding X bit
+ */
+ ((val & PMP_LOCK) != PMP_LOCK &&
+ (val & 0x7) != (PMP_WRITE | PMP_EXEC)))) ||
+ /*
+ * mseccfg.MML is not set
+ */
+ (!MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index))
+ ){
env->pmp_state.pmp[pmp_index].cfg_reg = val;
pmp_update_rule(env, pmp_index);
} else {
@@ -230,6 +271,18 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
/* Short cut if no rules */
if (0 == pmp_get_num_rules(env)) {
+ if (MSECCFG_MMWP_ISSET(env)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - %c mode access denied\n",
+ mode_to_char(mode));
+ return false;
+ }
+ if (MSECCFG_MML_ISSET(env) && (mode != PRV_M || (privs & PMP_EXEC))) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - %c mode access denied\n",
+ mode_to_char(mode));
+ return false;
+ }
return true;
}
@@ -265,16 +318,65 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
const uint8_t a_field =
pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
- /*
- * If the PMP entry is not off and the address is in range, do the priv
- * check
- */
if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
- allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
- if ((mode != PRV_M) || pmp_is_locked(env, i)) {
- allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
+ /*
+ * If the PMP entry is not off and the address is in range,
+ * do the priv check
+ */
+ if (!MSECCFG_MML_ISSET(env)) {
+ /*
+ * If mseccfg.MML Bit is not set, do pmp priv check
+ */
+ allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
+ if ((mode != PRV_M) || pmp_is_locked(env, i)) {
+ allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
+ }
+ } else {
+ /*
+ * If mseccfg.MML Bit set, do the enhanced pmp priv check
+ */
+ if (env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) {
+ /*
+ * Shared Region
+ */
+ if ((env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
+ allowed_privs = PMP_EXEC | ((mode == PRV_M &&
+ (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
+ PMP_READ : 0);
+ } else {
+ allowed_privs = env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE | PMP_EXEC);
+
+ if (mode != PRV_M && allowed_privs) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - %c mode access denied\n",
+ mode_to_char(mode));
+ ret = 0;
+ break;
+ }
+ }
+ } else {
+ /*
+ * Shared Region
+ */
+ if ((env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
+ allowed_privs = PMP_READ | ((mode == PRV_M ||
+ (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
+ PMP_WRITE : 0);
+ } else {
+ allowed_privs = env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE | PMP_EXEC);
+ if (mode == PRV_M && allowed_privs) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - m mode access denied\n");
+ ret = 0;
+ break;
+ }
+ }
+ }
}
-
if ((privs & allowed_privs) == privs) {
ret = 1;
break;
@@ -288,15 +390,23 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
/* No rule matched */
if (ret == -1) {
if (mode == PRV_M) {
- ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
- * M-Mode access, the access succeeds */
+ ret = !MSECCFG_MMWP_ISSET(env); /* PMP Enhancements */
+ if (MSECCFG_MML_ISSET(env) && (privs & PMP_EXEC)) {
+ ret = 0;
+ }
} else {
ret = 0; /* Other modes are not allowed to succeed if they don't
* match a rule, but there are rules. We've checked for
* no rule earlier in this function. */
}
}
-
+ if (ret) {
+ trace_pmp_hart_has_privs_pass_match(
+ env->mhartid, addr, size, privs, mode);
+ } else {
+ trace_pmp_hart_has_privs_violation(
+ env->mhartid, addr, size, privs, mode);
+ }
return ret == 1 ? true : false;
}
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index 8e19793132..7db2069204 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -36,6 +36,12 @@ typedef enum {
PMP_AMATCH_NAPOT /* Naturally aligned power-of-two region */
} pmp_am_t;
+typedef enum {
+ MSECCFG_MML = 1 << 0,
+ MSECCFG_MMWP = 1 << 1,
+ MSECCFG_RLB = 1 << 2
+} mseccfg_field_t;
+
typedef struct {
target_ulong addr_reg;
uint8_t cfg_reg;
@@ -58,7 +64,13 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
target_ulong val);
target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
+void mseccfg_csr_write(CPURISCVState *env, target_ulong val);
+target_ulong mseccfg_csr_read(CPURISCVState *env);
bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
target_ulong size, pmp_priv_t priv, target_ulong mode);
+#define MSECCFG_MML_ISSET(env) get_field(env->mseccfg, MSECCFG_MML)
+#define MSECCFG_MMWP_ISSET(env) get_field(env->mseccfg, MSECCFG_MMWP)
+#define MSECCFG_RLB_ISSET(env) get_field(env->mseccfg, MSECCFG_RLB)
+
#endif
diff --git a/target/riscv/trace-events b/target/riscv/trace-events
index 4b6c652ae9..4f877f90f7 100644
--- a/target/riscv/trace-events
+++ b/target/riscv/trace-events
@@ -6,3 +6,7 @@ pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRI
pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
pmpaddr_csr_read(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": read addr%" PRIu32", val: 0x%" PRIx64
pmpaddr_csr_write(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": write addr%" PRIu32", val: 0x%" PRIx64
+mseccfg_csr_read(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": read mseccfg, val: 0x%" PRIx64
+mseccfg_csr_write(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": write mseccfg, val: 0x%" PRIx64
+pmp_hart_has_privs_pass_match(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
+pmp_hart_has_privs_violation(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/4] Implementation of enhanced PMP(ePMP) support
@ 2020-08-11 0:23 ` Hou Weiying
0 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: palmer, Alistair.Francis, sagark, kbastian, Hongzheng-Li, Myriad-Dreamin
The ePMP can be found in:
https://docs.google.com/document/d/1Mh_aiHYxemL0umN3GTTw8vsbmzHZ_nxZXgjgOUzbvc8/edit#heading=h.9wsr1lnxtwe2
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/pmp.c | 134 ++++++++++++++++++++++++++++++++++----
target/riscv/pmp.h | 12 ++++
target/riscv/trace-events | 4 ++
3 files changed, 138 insertions(+), 12 deletions(-)
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 2a2b9f5363..b1fa703aff 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -34,6 +34,26 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index);
+static char mode_to_char(int mode)
+{
+ char ret = 0;
+ switch (mode) {
+ case PRV_U:
+ ret = 'u';
+ break;
+ case PRV_S:
+ ret = 's';
+ break;
+ case PRV_H:
+ ret = 'h';
+ break;
+ case PRV_M:
+ ret = 'm';
+ break;
+ }
+ return ret;
+}
+
/*
* Accessor method to extract address matching type 'a field' from cfg reg
*/
@@ -99,7 +119,28 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t pmp_index)
static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
{
if (pmp_index < MAX_RISCV_PMPS) {
- if (!pmp_is_locked(env, pmp_index)) {
+ /*
+ * mseccfg.RLB is set
+ */
+ if (MSECCFG_RLB_ISSET(env) ||
+ /*
+ * mseccfg.MML is set
+ */
+ (MSECCFG_MML_ISSET(env) &&
+ /*
+ * m model and not adding X bit
+ */
+ (((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) ||
+ /*
+ * shared region and not adding X bit
+ */
+ ((val & PMP_LOCK) != PMP_LOCK &&
+ (val & 0x7) != (PMP_WRITE | PMP_EXEC)))) ||
+ /*
+ * mseccfg.MML is not set
+ */
+ (!MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index))
+ ){
env->pmp_state.pmp[pmp_index].cfg_reg = val;
pmp_update_rule(env, pmp_index);
} else {
@@ -230,6 +271,18 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
/* Short cut if no rules */
if (0 == pmp_get_num_rules(env)) {
+ if (MSECCFG_MMWP_ISSET(env)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - %c mode access denied\n",
+ mode_to_char(mode));
+ return false;
+ }
+ if (MSECCFG_MML_ISSET(env) && (mode != PRV_M || (privs & PMP_EXEC))) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - %c mode access denied\n",
+ mode_to_char(mode));
+ return false;
+ }
return true;
}
@@ -265,16 +318,65 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
const uint8_t a_field =
pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
- /*
- * If the PMP entry is not off and the address is in range, do the priv
- * check
- */
if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
- allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
- if ((mode != PRV_M) || pmp_is_locked(env, i)) {
- allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
+ /*
+ * If the PMP entry is not off and the address is in range,
+ * do the priv check
+ */
+ if (!MSECCFG_MML_ISSET(env)) {
+ /*
+ * If mseccfg.MML Bit is not set, do pmp priv check
+ */
+ allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
+ if ((mode != PRV_M) || pmp_is_locked(env, i)) {
+ allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
+ }
+ } else {
+ /*
+ * If mseccfg.MML Bit set, do the enhanced pmp priv check
+ */
+ if (env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) {
+ /*
+ * Shared Region
+ */
+ if ((env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
+ allowed_privs = PMP_EXEC | ((mode == PRV_M &&
+ (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
+ PMP_READ : 0);
+ } else {
+ allowed_privs = env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE | PMP_EXEC);
+
+ if (mode != PRV_M && allowed_privs) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - %c mode access denied\n",
+ mode_to_char(mode));
+ ret = 0;
+ break;
+ }
+ }
+ } else {
+ /*
+ * Shared Region
+ */
+ if ((env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
+ allowed_privs = PMP_READ | ((mode == PRV_M ||
+ (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
+ PMP_WRITE : 0);
+ } else {
+ allowed_privs = env->pmp_state.pmp[i].cfg_reg &
+ (PMP_READ | PMP_WRITE | PMP_EXEC);
+ if (mode == PRV_M && allowed_privs) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pmp violation - m mode access denied\n");
+ ret = 0;
+ break;
+ }
+ }
+ }
}
-
if ((privs & allowed_privs) == privs) {
ret = 1;
break;
@@ -288,15 +390,23 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
/* No rule matched */
if (ret == -1) {
if (mode == PRV_M) {
- ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
- * M-Mode access, the access succeeds */
+ ret = !MSECCFG_MMWP_ISSET(env); /* PMP Enhancements */
+ if (MSECCFG_MML_ISSET(env) && (privs & PMP_EXEC)) {
+ ret = 0;
+ }
} else {
ret = 0; /* Other modes are not allowed to succeed if they don't
* match a rule, but there are rules. We've checked for
* no rule earlier in this function. */
}
}
-
+ if (ret) {
+ trace_pmp_hart_has_privs_pass_match(
+ env->mhartid, addr, size, privs, mode);
+ } else {
+ trace_pmp_hart_has_privs_violation(
+ env->mhartid, addr, size, privs, mode);
+ }
return ret == 1 ? true : false;
}
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index 8e19793132..7db2069204 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -36,6 +36,12 @@ typedef enum {
PMP_AMATCH_NAPOT /* Naturally aligned power-of-two region */
} pmp_am_t;
+typedef enum {
+ MSECCFG_MML = 1 << 0,
+ MSECCFG_MMWP = 1 << 1,
+ MSECCFG_RLB = 1 << 2
+} mseccfg_field_t;
+
typedef struct {
target_ulong addr_reg;
uint8_t cfg_reg;
@@ -58,7 +64,13 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
target_ulong val);
target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
+void mseccfg_csr_write(CPURISCVState *env, target_ulong val);
+target_ulong mseccfg_csr_read(CPURISCVState *env);
bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
target_ulong size, pmp_priv_t priv, target_ulong mode);
+#define MSECCFG_MML_ISSET(env) get_field(env->mseccfg, MSECCFG_MML)
+#define MSECCFG_MMWP_ISSET(env) get_field(env->mseccfg, MSECCFG_MMWP)
+#define MSECCFG_RLB_ISSET(env) get_field(env->mseccfg, MSECCFG_RLB)
+
#endif
diff --git a/target/riscv/trace-events b/target/riscv/trace-events
index 4b6c652ae9..4f877f90f7 100644
--- a/target/riscv/trace-events
+++ b/target/riscv/trace-events
@@ -6,3 +6,7 @@ pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRI
pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
pmpaddr_csr_read(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": read addr%" PRIu32", val: 0x%" PRIx64
pmpaddr_csr_write(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": write addr%" PRIu32", val: 0x%" PRIx64
+mseccfg_csr_read(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": read mseccfg, val: 0x%" PRIx64
+mseccfg_csr_write(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": write mseccfg, val: 0x%" PRIx64
+pmp_hart_has_privs_pass_match(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
+pmp_hart_has_privs_violation(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/4] Add ePMP CSR accesses
[not found] <20200811002325.46056-1-weiying_hou@outlook.com>
@ 2020-08-11 0:23 ` Hou Weiying
2020-08-11 0:23 ` Hou Weiying
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: sagark, kbastian, Hongzheng-Li, Alistair.Francis, Myriad-Dreamin, palmer
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/csr.c | 18 ++++++++++++++++++
target/riscv/pmp.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6a96a01b1c..0bb33baec3 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -104,6 +104,11 @@ static int hmode(CPURISCVState *env, int csrno)
return -1;
}
+static int epmp(CPURISCVState *env, int csrno)
+{
+ return -!(env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP));
+}
+
static int pmp(CPURISCVState *env, int csrno)
{
return -!riscv_feature(env, RISCV_FEATURE_PMP);
@@ -1142,6 +1147,18 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
return 0;
}
+static int read_mseccfg(CPURISCVState *env, int csrno, target_ulong *val)
+{
+ *val = mseccfg_csr_read(env);
+ return 0;
+}
+
+static int write_mseccfg(CPURISCVState *env, int csrno, target_ulong val)
+{
+ mseccfg_csr_write(env, val);
+ return 0;
+}
+
#endif
/*
@@ -1353,6 +1370,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MTINST] = { hmode, read_mtinst, write_mtinst },
/* Physical Memory Protection */
+ [CSR_MSECCFG] = { epmp, read_mseccfg, write_mseccfg },
[CSR_PMPCFG0 ... CSR_PMPCFG3] = { pmp, read_pmpcfg, write_pmpcfg },
[CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp, read_pmpaddr, write_pmpaddr },
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index b1fa703aff..97aab0b99e 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -494,3 +494,43 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
return val;
}
+
+
+/*
+ * Handle a write to a mseccfg CSR
+ */
+void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
+{
+ int i;
+
+ if (!MSECCFG_RLB_ISSET(env)) {
+ for (i = 0; i < MAX_RISCV_PMPS; i++) {
+ if (pmp_is_locked(env, i)) {
+ /*
+ * Now that mseccfg.rlb is zero
+ * the value of mseccfg.rlb should be locked.
+ */
+ val &= ~MSECCFG_RLB;
+ break;
+ }
+ }
+ }
+
+ /*
+ * sticky bit
+ */
+ val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
+
+ env->mseccfg = val;
+ trace_mseccfg_csr_write(env->mhartid, val);
+}
+
+
+/*
+ * Handle a read from a mseccfg CSR
+ */
+target_ulong mseccfg_csr_read(CPURISCVState *env)
+{
+ trace_mseccfg_csr_read(env->mhartid, env->mseccfg);
+ return env->mseccfg;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/4] Add ePMP CSR accesses
@ 2020-08-11 0:23 ` Hou Weiying
0 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: palmer, Alistair.Francis, sagark, kbastian, Hongzheng-Li, Myriad-Dreamin
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/csr.c | 18 ++++++++++++++++++
target/riscv/pmp.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6a96a01b1c..0bb33baec3 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -104,6 +104,11 @@ static int hmode(CPURISCVState *env, int csrno)
return -1;
}
+static int epmp(CPURISCVState *env, int csrno)
+{
+ return -!(env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP));
+}
+
static int pmp(CPURISCVState *env, int csrno)
{
return -!riscv_feature(env, RISCV_FEATURE_PMP);
@@ -1142,6 +1147,18 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
return 0;
}
+static int read_mseccfg(CPURISCVState *env, int csrno, target_ulong *val)
+{
+ *val = mseccfg_csr_read(env);
+ return 0;
+}
+
+static int write_mseccfg(CPURISCVState *env, int csrno, target_ulong val)
+{
+ mseccfg_csr_write(env, val);
+ return 0;
+}
+
#endif
/*
@@ -1353,6 +1370,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MTINST] = { hmode, read_mtinst, write_mtinst },
/* Physical Memory Protection */
+ [CSR_MSECCFG] = { epmp, read_mseccfg, write_mseccfg },
[CSR_PMPCFG0 ... CSR_PMPCFG3] = { pmp, read_pmpcfg, write_pmpcfg },
[CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp, read_pmpaddr, write_pmpaddr },
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index b1fa703aff..97aab0b99e 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -494,3 +494,43 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
return val;
}
+
+
+/*
+ * Handle a write to a mseccfg CSR
+ */
+void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
+{
+ int i;
+
+ if (!MSECCFG_RLB_ISSET(env)) {
+ for (i = 0; i < MAX_RISCV_PMPS; i++) {
+ if (pmp_is_locked(env, i)) {
+ /*
+ * Now that mseccfg.rlb is zero
+ * the value of mseccfg.rlb should be locked.
+ */
+ val &= ~MSECCFG_RLB;
+ break;
+ }
+ }
+ }
+
+ /*
+ * sticky bit
+ */
+ val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
+
+ env->mseccfg = val;
+ trace_mseccfg_csr_write(env->mhartid, val);
+}
+
+
+/*
+ * Handle a read from a mseccfg CSR
+ */
+target_ulong mseccfg_csr_read(CPURISCVState *env)
+{
+ trace_mseccfg_csr_read(env->mhartid, env->mseccfg);
+ return env->mseccfg;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/4] Add a config option for ePMP.
[not found] <20200811002325.46056-1-weiying_hou@outlook.com>
@ 2020-08-11 0:23 ` Hou Weiying
2020-08-11 0:23 ` Hou Weiying
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: sagark, kbastian, Hongzheng-Li, Alistair.Francis, Myriad-Dreamin, palmer
Add a config option to enable experimental support for ePMP. This
is disabled by default and can be enabled with 'x-epmp=true'.
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/cpu.c | 9 +++++++++
target/riscv/cpu.h | 3 +++
2 files changed, 12 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 228b9bdb5d..d691d6ffd6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -371,6 +371,14 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
if (cpu->cfg.pmp) {
set_feature(env, RISCV_FEATURE_PMP);
+
+ /*
+ * Enhanced PMP should only be available
+ * on harts with PMP support
+ */
+ if (cpu->cfg.epmp) {
+ set_feature(env, RISCV_FEATURE_EPMP);
+ }
}
/* If misa isn't set (rv32 and rv64 machines) set it here */
@@ -518,6 +526,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
+ DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a804a5d0ba..9a813f3f1c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -73,6 +73,7 @@
enum {
RISCV_FEATURE_MMU,
RISCV_FEATURE_PMP,
+ RISCV_FEATURE_EPMP,
RISCV_FEATURE_MISA
};
@@ -217,6 +218,7 @@ struct CPURISCVState {
/* physical memory protection */
pmp_table_t pmp_state;
+ target_ulong mseccfg;
/* machine specific rdtime callback */
uint64_t (*rdtime_fn)(void);
@@ -291,6 +293,7 @@ typedef struct RISCVCPU {
uint16_t elen;
bool mmu;
bool pmp;
+ bool epmp;
} cfg;
} RISCVCPU;
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/4] Add a config option for ePMP.
@ 2020-08-11 0:23 ` Hou Weiying
0 siblings, 0 replies; 16+ messages in thread
From: Hou Weiying @ 2020-08-11 0:23 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: palmer, Alistair.Francis, sagark, kbastian, Hongzheng-Li, Myriad-Dreamin
Add a config option to enable experimental support for ePMP. This
is disabled by default and can be enabled with 'x-epmp=true'.
Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
---
target/riscv/cpu.c | 9 +++++++++
target/riscv/cpu.h | 3 +++
2 files changed, 12 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 228b9bdb5d..d691d6ffd6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -371,6 +371,14 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
if (cpu->cfg.pmp) {
set_feature(env, RISCV_FEATURE_PMP);
+
+ /*
+ * Enhanced PMP should only be available
+ * on harts with PMP support
+ */
+ if (cpu->cfg.epmp) {
+ set_feature(env, RISCV_FEATURE_EPMP);
+ }
}
/* If misa isn't set (rv32 and rv64 machines) set it here */
@@ -518,6 +526,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
+ DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a804a5d0ba..9a813f3f1c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -73,6 +73,7 @@
enum {
RISCV_FEATURE_MMU,
RISCV_FEATURE_PMP,
+ RISCV_FEATURE_EPMP,
RISCV_FEATURE_MISA
};
@@ -217,6 +218,7 @@ struct CPURISCVState {
/* physical memory protection */
pmp_table_t pmp_state;
+ target_ulong mseccfg;
/* machine specific rdtime callback */
uint64_t (*rdtime_fn)(void);
@@ -291,6 +293,7 @@ typedef struct RISCVCPU {
uint16_t elen;
bool mmu;
bool pmp;
+ bool epmp;
} cfg;
} RISCVCPU;
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] Define ePMP mseccfg
2020-08-11 0:23 ` Hou Weiying
@ 2020-08-25 22:31 ` Alistair Francis
-1 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2020-08-25 22:31 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Hongzheng-Li, Alistair Francis,
Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:25 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> Currently using 0x390 and 0x391 for x-epmp (experimental). This may change in the future spec.
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
> ---
> target/riscv/cpu_bits.h | 3 +++
> target/riscv/gdbstub.c | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 8117e8b5a7..9c35179983 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -229,6 +229,9 @@
> #define CSR_MTINST 0x34a
> #define CSR_MTVAL2 0x34b
>
> +/* Enhanced PMP */
> +#define CSR_MSECCFG 0x390
> +#define CSR_MSECCFGH 0x391
I was hoping that this address would be set by this time, but that
doesn't seem to have happened. I'll try and get this going.
I think we will have to wait for the address to be finalised before
this can be merged.
Alistair
> /* Physical Memory Protection */
> #define CSR_PMPCFG0 0x3a0
> #define CSR_PMPCFG1 0x3a1
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index eba12a86f2..de5551604a 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -132,6 +132,8 @@ static int csr_register_map[] = {
> CSR_MIP,
> CSR_MTINST,
> CSR_MTVAL2,
> + CSR_MSECCFG,
> + CSR_MSECCFGH,
> CSR_PMPCFG0,
> CSR_PMPCFG1,
> CSR_PMPCFG2,
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] Define ePMP mseccfg
@ 2020-08-25 22:31 ` Alistair Francis
0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2020-08-25 22:31 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers,
Sagar Karandikar, Bastian Koppelmann, Hongzheng-Li,
Alistair Francis, Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:25 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> Currently using 0x390 and 0x391 for x-epmp (experimental). This may change in the future spec.
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
> ---
> target/riscv/cpu_bits.h | 3 +++
> target/riscv/gdbstub.c | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 8117e8b5a7..9c35179983 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -229,6 +229,9 @@
> #define CSR_MTINST 0x34a
> #define CSR_MTVAL2 0x34b
>
> +/* Enhanced PMP */
> +#define CSR_MSECCFG 0x390
> +#define CSR_MSECCFGH 0x391
I was hoping that this address would be set by this time, but that
doesn't seem to have happened. I'll try and get this going.
I think we will have to wait for the address to be finalised before
this can be merged.
Alistair
> /* Physical Memory Protection */
> #define CSR_PMPCFG0 0x3a0
> #define CSR_PMPCFG1 0x3a1
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index eba12a86f2..de5551604a 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -132,6 +132,8 @@ static int csr_register_map[] = {
> CSR_MIP,
> CSR_MTINST,
> CSR_MTVAL2,
> + CSR_MSECCFG,
> + CSR_MSECCFGH,
> CSR_PMPCFG0,
> CSR_PMPCFG1,
> CSR_PMPCFG2,
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/4] Implementation of enhanced PMP(ePMP) support
2020-08-11 0:23 ` Hou Weiying
@ 2021-02-09 18:25 ` Alistair Francis
-1 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2021-02-09 18:25 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Hongzheng-Li, Alistair Francis,
Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:24 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> The ePMP can be found in:
> https://docs.google.com/document/d/1Mh_aiHYxemL0umN3GTTw8vsbmzHZ_nxZXgjgOUzbvc8/edit#heading=h.9wsr1lnxtwe2
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
Thanks for the patch!
I haven't forgotten about this. I have been hoping that the CSR
addresses would be finalised so then this can be merged. Unfortunately
that still hasn't happened!
> ---
> target/riscv/pmp.c | 134 ++++++++++++++++++++++++++++++++++----
> target/riscv/pmp.h | 12 ++++
> target/riscv/trace-events | 4 ++
> 3 files changed, 138 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a2b9f5363..b1fa703aff 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -34,6 +34,26 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
> static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
> static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index);
>
> +static char mode_to_char(int mode)
> +{
> + char ret = 0;
> + switch (mode) {
> + case PRV_U:
> + ret = 'u';
> + break;
> + case PRV_S:
> + ret = 's';
> + break;
> + case PRV_H:
> + ret = 'h';
> + break;
> + case PRV_M:
> + ret = 'm';
> + break;
> + }
> + return ret;
> +}
> +
> /*
> * Accessor method to extract address matching type 'a field' from cfg reg
> */
> @@ -99,7 +119,28 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t pmp_index)
> static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
> {
> if (pmp_index < MAX_RISCV_PMPS) {
> - if (!pmp_is_locked(env, pmp_index)) {
> + /*
> + * mseccfg.RLB is set
> + */
These can just be single line comments instead of three.
> + if (MSECCFG_RLB_ISSET(env) ||
> + /*
> + * mseccfg.MML is set
> + */
> + (MSECCFG_MML_ISSET(env) &&
> + /*
> + * m model and not adding X bit
> + */
> + (((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) ||
> + /*
> + * shared region and not adding X bit
> + */
> + ((val & PMP_LOCK) != PMP_LOCK &&
> + (val & 0x7) != (PMP_WRITE | PMP_EXEC)))) ||
> + /*
> + * mseccfg.MML is not set
> + */
> + (!MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index))
> + ){
> env->pmp_state.pmp[pmp_index].cfg_reg = val;
> pmp_update_rule(env, pmp_index);
> } else {
> @@ -230,6 +271,18 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>
> /* Short cut if no rules */
> if (0 == pmp_get_num_rules(env)) {
> + if (MSECCFG_MMWP_ISSET(env)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> + if (MSECCFG_MML_ISSET(env) && (mode != PRV_M || (privs & PMP_EXEC))) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> return true;
> }
>
> @@ -265,16 +318,65 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> const uint8_t a_field =
> pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
>
> - /*
> - * If the PMP entry is not off and the address is in range, do the priv
> - * check
> - */
> if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
> - allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> - if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> - allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + /*
> + * If the PMP entry is not off and the address is in range,
> + * do the priv check
> + */
> + if (!MSECCFG_MML_ISSET(env)) {
> + /*
> + * If mseccfg.MML Bit is not set, do pmp priv check
> + */
> + allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> + if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> + allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + }
> + } else {
> + /*
> + * If mseccfg.MML Bit set, do the enhanced pmp priv check
> + */
> + if (env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_EXEC | ((mode == PRV_M &&
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_READ : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> +
> + if (mode != PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + ret = 0;
> + break;
> + }
> + }
> + } else {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_READ | ((mode == PRV_M ||
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_WRITE : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> + if (mode == PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - m mode access denied\n");
> + ret = 0;
> + break;
> + }
> + }
> + }
> }
> -
> if ((privs & allowed_privs) == privs) {
> ret = 1;
> break;
> @@ -288,15 +390,23 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> /* No rule matched */
> if (ret == -1) {
> if (mode == PRV_M) {
> - ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
> - * M-Mode access, the access succeeds */
> + ret = !MSECCFG_MMWP_ISSET(env); /* PMP Enhancements */
> + if (MSECCFG_MML_ISSET(env) && (privs & PMP_EXEC)) {
> + ret = 0;
> + }
> } else {
> ret = 0; /* Other modes are not allowed to succeed if they don't
> * match a rule, but there are rules. We've checked for
> * no rule earlier in this function. */
> }
> }
> -
> + if (ret) {
> + trace_pmp_hart_has_privs_pass_match(
> + env->mhartid, addr, size, privs, mode);
> + } else {
> + trace_pmp_hart_has_privs_violation(
> + env->mhartid, addr, size, privs, mode);
> + }
> return ret == 1 ? true : false;
> }
>
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index 8e19793132..7db2069204 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -36,6 +36,12 @@ typedef enum {
> PMP_AMATCH_NAPOT /* Naturally aligned power-of-two region */
> } pmp_am_t;
>
> +typedef enum {
> + MSECCFG_MML = 1 << 0,
> + MSECCFG_MMWP = 1 << 1,
> + MSECCFG_RLB = 1 << 2
> +} mseccfg_field_t;
> +
> typedef struct {
> target_ulong addr_reg;
> uint8_t cfg_reg;
> @@ -58,7 +64,13 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
> void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
> target_ulong val);
> target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
> +void mseccfg_csr_write(CPURISCVState *env, target_ulong val);
> +target_ulong mseccfg_csr_read(CPURISCVState *env);
> bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> target_ulong size, pmp_priv_t priv, target_ulong mode);
>
> +#define MSECCFG_MML_ISSET(env) get_field(env->mseccfg, MSECCFG_MML)
> +#define MSECCFG_MMWP_ISSET(env) get_field(env->mseccfg, MSECCFG_MMWP)
> +#define MSECCFG_RLB_ISSET(env) get_field(env->mseccfg, MSECCFG_RLB)
This fails to compile as the `target_ulong mseccfg` variable has not
been added in this patch.
Alistair
> +
> #endif
> diff --git a/target/riscv/trace-events b/target/riscv/trace-events
> index 4b6c652ae9..4f877f90f7 100644
> --- a/target/riscv/trace-events
> +++ b/target/riscv/trace-events
> @@ -6,3 +6,7 @@ pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRI
> pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_read(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": read addr%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_write(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": write addr%" PRIu32", val: 0x%" PRIx64
> +mseccfg_csr_read(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": read mseccfg, val: 0x%" PRIx64
> +mseccfg_csr_write(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": write mseccfg, val: 0x%" PRIx64
> +pmp_hart_has_privs_pass_match(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> +pmp_hart_has_privs_violation(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/4] Implementation of enhanced PMP(ePMP) support
@ 2021-02-09 18:25 ` Alistair Francis
0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2021-02-09 18:25 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers,
Sagar Karandikar, Bastian Koppelmann, Hongzheng-Li,
Alistair Francis, Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:24 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> The ePMP can be found in:
> https://docs.google.com/document/d/1Mh_aiHYxemL0umN3GTTw8vsbmzHZ_nxZXgjgOUzbvc8/edit#heading=h.9wsr1lnxtwe2
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
Thanks for the patch!
I haven't forgotten about this. I have been hoping that the CSR
addresses would be finalised so then this can be merged. Unfortunately
that still hasn't happened!
> ---
> target/riscv/pmp.c | 134 ++++++++++++++++++++++++++++++++++----
> target/riscv/pmp.h | 12 ++++
> target/riscv/trace-events | 4 ++
> 3 files changed, 138 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a2b9f5363..b1fa703aff 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -34,6 +34,26 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
> static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
> static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index);
>
> +static char mode_to_char(int mode)
> +{
> + char ret = 0;
> + switch (mode) {
> + case PRV_U:
> + ret = 'u';
> + break;
> + case PRV_S:
> + ret = 's';
> + break;
> + case PRV_H:
> + ret = 'h';
> + break;
> + case PRV_M:
> + ret = 'm';
> + break;
> + }
> + return ret;
> +}
> +
> /*
> * Accessor method to extract address matching type 'a field' from cfg reg
> */
> @@ -99,7 +119,28 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t pmp_index)
> static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
> {
> if (pmp_index < MAX_RISCV_PMPS) {
> - if (!pmp_is_locked(env, pmp_index)) {
> + /*
> + * mseccfg.RLB is set
> + */
These can just be single line comments instead of three.
> + if (MSECCFG_RLB_ISSET(env) ||
> + /*
> + * mseccfg.MML is set
> + */
> + (MSECCFG_MML_ISSET(env) &&
> + /*
> + * m model and not adding X bit
> + */
> + (((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) ||
> + /*
> + * shared region and not adding X bit
> + */
> + ((val & PMP_LOCK) != PMP_LOCK &&
> + (val & 0x7) != (PMP_WRITE | PMP_EXEC)))) ||
> + /*
> + * mseccfg.MML is not set
> + */
> + (!MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index))
> + ){
> env->pmp_state.pmp[pmp_index].cfg_reg = val;
> pmp_update_rule(env, pmp_index);
> } else {
> @@ -230,6 +271,18 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>
> /* Short cut if no rules */
> if (0 == pmp_get_num_rules(env)) {
> + if (MSECCFG_MMWP_ISSET(env)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> + if (MSECCFG_MML_ISSET(env) && (mode != PRV_M || (privs & PMP_EXEC))) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> return true;
> }
>
> @@ -265,16 +318,65 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> const uint8_t a_field =
> pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
>
> - /*
> - * If the PMP entry is not off and the address is in range, do the priv
> - * check
> - */
> if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
> - allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> - if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> - allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + /*
> + * If the PMP entry is not off and the address is in range,
> + * do the priv check
> + */
> + if (!MSECCFG_MML_ISSET(env)) {
> + /*
> + * If mseccfg.MML Bit is not set, do pmp priv check
> + */
> + allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> + if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> + allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + }
> + } else {
> + /*
> + * If mseccfg.MML Bit set, do the enhanced pmp priv check
> + */
> + if (env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_EXEC | ((mode == PRV_M &&
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_READ : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> +
> + if (mode != PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + ret = 0;
> + break;
> + }
> + }
> + } else {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_READ | ((mode == PRV_M ||
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_WRITE : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> + if (mode == PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - m mode access denied\n");
> + ret = 0;
> + break;
> + }
> + }
> + }
> }
> -
> if ((privs & allowed_privs) == privs) {
> ret = 1;
> break;
> @@ -288,15 +390,23 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> /* No rule matched */
> if (ret == -1) {
> if (mode == PRV_M) {
> - ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
> - * M-Mode access, the access succeeds */
> + ret = !MSECCFG_MMWP_ISSET(env); /* PMP Enhancements */
> + if (MSECCFG_MML_ISSET(env) && (privs & PMP_EXEC)) {
> + ret = 0;
> + }
> } else {
> ret = 0; /* Other modes are not allowed to succeed if they don't
> * match a rule, but there are rules. We've checked for
> * no rule earlier in this function. */
> }
> }
> -
> + if (ret) {
> + trace_pmp_hart_has_privs_pass_match(
> + env->mhartid, addr, size, privs, mode);
> + } else {
> + trace_pmp_hart_has_privs_violation(
> + env->mhartid, addr, size, privs, mode);
> + }
> return ret == 1 ? true : false;
> }
>
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index 8e19793132..7db2069204 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -36,6 +36,12 @@ typedef enum {
> PMP_AMATCH_NAPOT /* Naturally aligned power-of-two region */
> } pmp_am_t;
>
> +typedef enum {
> + MSECCFG_MML = 1 << 0,
> + MSECCFG_MMWP = 1 << 1,
> + MSECCFG_RLB = 1 << 2
> +} mseccfg_field_t;
> +
> typedef struct {
> target_ulong addr_reg;
> uint8_t cfg_reg;
> @@ -58,7 +64,13 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
> void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
> target_ulong val);
> target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
> +void mseccfg_csr_write(CPURISCVState *env, target_ulong val);
> +target_ulong mseccfg_csr_read(CPURISCVState *env);
> bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> target_ulong size, pmp_priv_t priv, target_ulong mode);
>
> +#define MSECCFG_MML_ISSET(env) get_field(env->mseccfg, MSECCFG_MML)
> +#define MSECCFG_MMWP_ISSET(env) get_field(env->mseccfg, MSECCFG_MMWP)
> +#define MSECCFG_RLB_ISSET(env) get_field(env->mseccfg, MSECCFG_RLB)
This fails to compile as the `target_ulong mseccfg` variable has not
been added in this patch.
Alistair
> +
> #endif
> diff --git a/target/riscv/trace-events b/target/riscv/trace-events
> index 4b6c652ae9..4f877f90f7 100644
> --- a/target/riscv/trace-events
> +++ b/target/riscv/trace-events
> @@ -6,3 +6,7 @@ pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRI
> pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_read(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": read addr%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_write(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": write addr%" PRIu32", val: 0x%" PRIx64
> +mseccfg_csr_read(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": read mseccfg, val: 0x%" PRIx64
> +mseccfg_csr_write(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": write mseccfg, val: 0x%" PRIx64
> +pmp_hart_has_privs_pass_match(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> +pmp_hart_has_privs_violation(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] Add ePMP CSR accesses
2020-08-11 0:23 ` Hou Weiying
@ 2021-02-10 20:11 ` Alistair Francis
-1 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2021-02-10 20:11 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Hongzheng-Li, Alistair Francis,
Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:24 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
> ---
> target/riscv/csr.c | 18 ++++++++++++++++++
> target/riscv/pmp.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 58 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 6a96a01b1c..0bb33baec3 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -104,6 +104,11 @@ static int hmode(CPURISCVState *env, int csrno)
> return -1;
> }
>
> +static int epmp(CPURISCVState *env, int csrno)
> +{
> + return -!(env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP));
RISCV_FEATURE_EPMP isn't defined yet, so this fails to compile.
Alistair
> +}
> +
> static int pmp(CPURISCVState *env, int csrno)
> {
> return -!riscv_feature(env, RISCV_FEATURE_PMP);
> @@ -1142,6 +1147,18 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
> return 0;
> }
>
> +static int read_mseccfg(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> + *val = mseccfg_csr_read(env);
> + return 0;
> +}
> +
> +static int write_mseccfg(CPURISCVState *env, int csrno, target_ulong val)
> +{
> + mseccfg_csr_write(env, val);
> + return 0;
> +}
> +
> #endif
>
> /*
> @@ -1353,6 +1370,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MTINST] = { hmode, read_mtinst, write_mtinst },
>
> /* Physical Memory Protection */
> + [CSR_MSECCFG] = { epmp, read_mseccfg, write_mseccfg },
> [CSR_PMPCFG0 ... CSR_PMPCFG3] = { pmp, read_pmpcfg, write_pmpcfg },
> [CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp, read_pmpaddr, write_pmpaddr },
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index b1fa703aff..97aab0b99e 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -494,3 +494,43 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
>
> return val;
> }
> +
> +
> +/*
> + * Handle a write to a mseccfg CSR
> + */
> +void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
> +{
> + int i;
> +
> + if (!MSECCFG_RLB_ISSET(env)) {
> + for (i = 0; i < MAX_RISCV_PMPS; i++) {
> + if (pmp_is_locked(env, i)) {
> + /*
> + * Now that mseccfg.rlb is zero
> + * the value of mseccfg.rlb should be locked.
> + */
> + val &= ~MSECCFG_RLB;
> + break;
> + }
> + }
> + }
> +
> + /*
> + * sticky bit
> + */
> + val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> +
> + env->mseccfg = val;
> + trace_mseccfg_csr_write(env->mhartid, val);
> +}
> +
> +
> +/*
> + * Handle a read from a mseccfg CSR
> + */
> +target_ulong mseccfg_csr_read(CPURISCVState *env)
> +{
> + trace_mseccfg_csr_read(env->mhartid, env->mseccfg);
> + return env->mseccfg;
> +}
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] Add ePMP CSR accesses
@ 2021-02-10 20:11 ` Alistair Francis
0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2021-02-10 20:11 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers,
Sagar Karandikar, Bastian Koppelmann, Hongzheng-Li,
Alistair Francis, Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:24 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
> ---
> target/riscv/csr.c | 18 ++++++++++++++++++
> target/riscv/pmp.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 58 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 6a96a01b1c..0bb33baec3 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -104,6 +104,11 @@ static int hmode(CPURISCVState *env, int csrno)
> return -1;
> }
>
> +static int epmp(CPURISCVState *env, int csrno)
> +{
> + return -!(env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP));
RISCV_FEATURE_EPMP isn't defined yet, so this fails to compile.
Alistair
> +}
> +
> static int pmp(CPURISCVState *env, int csrno)
> {
> return -!riscv_feature(env, RISCV_FEATURE_PMP);
> @@ -1142,6 +1147,18 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
> return 0;
> }
>
> +static int read_mseccfg(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> + *val = mseccfg_csr_read(env);
> + return 0;
> +}
> +
> +static int write_mseccfg(CPURISCVState *env, int csrno, target_ulong val)
> +{
> + mseccfg_csr_write(env, val);
> + return 0;
> +}
> +
> #endif
>
> /*
> @@ -1353,6 +1370,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MTINST] = { hmode, read_mtinst, write_mtinst },
>
> /* Physical Memory Protection */
> + [CSR_MSECCFG] = { epmp, read_mseccfg, write_mseccfg },
> [CSR_PMPCFG0 ... CSR_PMPCFG3] = { pmp, read_pmpcfg, write_pmpcfg },
> [CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp, read_pmpaddr, write_pmpaddr },
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index b1fa703aff..97aab0b99e 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -494,3 +494,43 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
>
> return val;
> }
> +
> +
> +/*
> + * Handle a write to a mseccfg CSR
> + */
> +void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
> +{
> + int i;
> +
> + if (!MSECCFG_RLB_ISSET(env)) {
> + for (i = 0; i < MAX_RISCV_PMPS; i++) {
> + if (pmp_is_locked(env, i)) {
> + /*
> + * Now that mseccfg.rlb is zero
> + * the value of mseccfg.rlb should be locked.
> + */
> + val &= ~MSECCFG_RLB;
> + break;
> + }
> + }
> + }
> +
> + /*
> + * sticky bit
> + */
> + val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> +
> + env->mseccfg = val;
> + trace_mseccfg_csr_write(env->mhartid, val);
> +}
> +
> +
> +/*
> + * Handle a read from a mseccfg CSR
> + */
> +target_ulong mseccfg_csr_read(CPURISCVState *env)
> +{
> + trace_mseccfg_csr_read(env->mhartid, env->mseccfg);
> + return env->mseccfg;
> +}
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/4] Implementation of enhanced PMP(ePMP) support
2020-08-11 0:23 ` Hou Weiying
@ 2021-02-10 20:18 ` Alistair Francis
-1 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2021-02-10 20:18 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Hongzheng-Li, Alistair Francis,
Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:24 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> The ePMP can be found in:
> https://docs.google.com/document/d/1Mh_aiHYxemL0umN3GTTw8vsbmzHZ_nxZXgjgOUzbvc8/edit#heading=h.9wsr1lnxtwe2
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
> ---
> target/riscv/pmp.c | 134 ++++++++++++++++++++++++++++++++++----
> target/riscv/pmp.h | 12 ++++
> target/riscv/trace-events | 4 ++
> 3 files changed, 138 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a2b9f5363..b1fa703aff 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -34,6 +34,26 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
> static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
> static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index);
>
> +static char mode_to_char(int mode)
> +{
> + char ret = 0;
> + switch (mode) {
> + case PRV_U:
> + ret = 'u';
> + break;
> + case PRV_S:
> + ret = 's';
> + break;
> + case PRV_H:
> + ret = 'h';
> + break;
> + case PRV_M:
> + ret = 'm';
> + break;
> + }
> + return ret;
> +}
> +
> /*
> * Accessor method to extract address matching type 'a field' from cfg reg
> */
> @@ -99,7 +119,28 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t pmp_index)
> static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
> {
> if (pmp_index < MAX_RISCV_PMPS) {
> - if (!pmp_is_locked(env, pmp_index)) {
> + /*
> + * mseccfg.RLB is set
> + */
> + if (MSECCFG_RLB_ISSET(env) ||
> + /*
> + * mseccfg.MML is set
> + */
> + (MSECCFG_MML_ISSET(env) &&
> + /*
> + * m model and not adding X bit
> + */
> + (((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) ||
> + /*
> + * shared region and not adding X bit
> + */
> + ((val & PMP_LOCK) != PMP_LOCK &&
> + (val & 0x7) != (PMP_WRITE | PMP_EXEC)))) ||
> + /*
> + * mseccfg.MML is not set
> + */
> + (!MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index))
> + ){
> env->pmp_state.pmp[pmp_index].cfg_reg = val;
> pmp_update_rule(env, pmp_index);
> } else {
> @@ -230,6 +271,18 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>
> /* Short cut if no rules */
> if (0 == pmp_get_num_rules(env)) {
> + if (MSECCFG_MMWP_ISSET(env)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> + if (MSECCFG_MML_ISSET(env) && (mode != PRV_M || (privs & PMP_EXEC))) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> return true;
> }
>
> @@ -265,16 +318,65 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> const uint8_t a_field =
> pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
>
> - /*
> - * If the PMP entry is not off and the address is in range, do the priv
> - * check
> - */
> if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
> - allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> - if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> - allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + /*
> + * If the PMP entry is not off and the address is in range,
> + * do the priv check
> + */
> + if (!MSECCFG_MML_ISSET(env)) {
> + /*
> + * If mseccfg.MML Bit is not set, do pmp priv check
> + */
> + allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> + if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> + allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + }
> + } else {
> + /*
> + * If mseccfg.MML Bit set, do the enhanced pmp priv check
> + */
> + if (env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_EXEC | ((mode == PRV_M &&
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_READ : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> +
> + if (mode != PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + ret = 0;
> + break;
> + }
> + }
> + } else {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_READ | ((mode == PRV_M ||
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_WRITE : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> + if (mode == PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - m mode access denied\n");
> + ret = 0;
> + break;
This is really hard to follow, I had a lot of trouble wrapping my head
around this and I'm still not sure it's correct.
I feel that a switch satement here would be easier. The spec has a
nice table of all the possible options, for example something like
this:
if (mode == PRV_M) {
switch (epmp_operation) {
case 0:
case 1:
case 4:
case 5:
case 6:
case 7:
case 8:
allowed_privs = 0;
break;
case 2:
case 3:
case 14:
allowed_privs = PMP_READ | PMP_WRITE;
break;
case 9:
case 10:
allowed_privs = PMP_EXEC;
break;
case 11:
case 13:
allowed_privs = PMP_READ | PMP_EXEC;
break;
case 12:
case 15:
allowed_privs = PMP_READ;
break;
}
} else {
switch (epmp_operation) {
case 0:
case 8:
case 9:
case 12:
case 13:
case 14:
allowed_privs = 0;
break;
case 1:
case 10:
case 11:
allowed_privs = PMP_EXEC;
break;
case 2:
case 4:
case 15:
allowed_privs = PMP_READ;
break;
case 3:
case 6:
allowed_privs = PMP_READ | PMP_WRITE;
break;
case 5:
allowed_privs = PMP_READ | PMP_EXEC;
break;
case 7:
allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
break;
}
}
It's a little more verbose, but it should still be fairly quick to compute.
If you want to keep the logic that's also fine, I just think it needs
some simplifiation. Maybe assign read, write, exec variables and just
compare them?
bool read = env->pmp_state.pmp[i].cfg_reg & PMP_READ;
bool write = env->pmp_state.pmp[i].cfg_reg & PMP_WRITE;
bool exec = env->pmp_state.pmp[i].cfg_reg & PMP_EXEC;
Then
if ((env->pmp_state.pmp[i].cfg_reg & (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
becomes
if (write && !read) {
Alistair
> + }
> + }
> + }
> }
> -
> if ((privs & allowed_privs) == privs) {
> ret = 1;
> break;
> @@ -288,15 +390,23 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> /* No rule matched */
> if (ret == -1) {
> if (mode == PRV_M) {
> - ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
> - * M-Mode access, the access succeeds */
> + ret = !MSECCFG_MMWP_ISSET(env); /* PMP Enhancements */
> + if (MSECCFG_MML_ISSET(env) && (privs & PMP_EXEC)) {
> + ret = 0;
> + }
> } else {
> ret = 0; /* Other modes are not allowed to succeed if they don't
> * match a rule, but there are rules. We've checked for
> * no rule earlier in this function. */
> }
> }
> -
> + if (ret) {
> + trace_pmp_hart_has_privs_pass_match(
> + env->mhartid, addr, size, privs, mode);
> + } else {
> + trace_pmp_hart_has_privs_violation(
> + env->mhartid, addr, size, privs, mode);
> + }
> return ret == 1 ? true : false;
> }
>
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index 8e19793132..7db2069204 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -36,6 +36,12 @@ typedef enum {
> PMP_AMATCH_NAPOT /* Naturally aligned power-of-two region */
> } pmp_am_t;
>
> +typedef enum {
> + MSECCFG_MML = 1 << 0,
> + MSECCFG_MMWP = 1 << 1,
> + MSECCFG_RLB = 1 << 2
> +} mseccfg_field_t;
> +
> typedef struct {
> target_ulong addr_reg;
> uint8_t cfg_reg;
> @@ -58,7 +64,13 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
> void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
> target_ulong val);
> target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
> +void mseccfg_csr_write(CPURISCVState *env, target_ulong val);
> +target_ulong mseccfg_csr_read(CPURISCVState *env);
> bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> target_ulong size, pmp_priv_t priv, target_ulong mode);
>
> +#define MSECCFG_MML_ISSET(env) get_field(env->mseccfg, MSECCFG_MML)
> +#define MSECCFG_MMWP_ISSET(env) get_field(env->mseccfg, MSECCFG_MMWP)
> +#define MSECCFG_RLB_ISSET(env) get_field(env->mseccfg, MSECCFG_RLB)
> +
> #endif
> diff --git a/target/riscv/trace-events b/target/riscv/trace-events
> index 4b6c652ae9..4f877f90f7 100644
> --- a/target/riscv/trace-events
> +++ b/target/riscv/trace-events
> @@ -6,3 +6,7 @@ pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRI
> pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_read(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": read addr%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_write(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": write addr%" PRIu32", val: 0x%" PRIx64
> +mseccfg_csr_read(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": read mseccfg, val: 0x%" PRIx64
> +mseccfg_csr_write(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": write mseccfg, val: 0x%" PRIx64
> +pmp_hart_has_privs_pass_match(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> +pmp_hart_has_privs_violation(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/4] Implementation of enhanced PMP(ePMP) support
@ 2021-02-10 20:18 ` Alistair Francis
0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2021-02-10 20:18 UTC (permalink / raw)
To: Hou Weiying
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers,
Sagar Karandikar, Bastian Koppelmann, Hongzheng-Li,
Alistair Francis, Myriad-Dreamin, Palmer Dabbelt
On Mon, Aug 10, 2020 at 5:24 PM Hou Weiying <weiying_hou@outlook.com> wrote:
>
> The ePMP can be found in:
> https://docs.google.com/document/d/1Mh_aiHYxemL0umN3GTTw8vsbmzHZ_nxZXgjgOUzbvc8/edit#heading=h.9wsr1lnxtwe2
>
> Signed-off-by: Hongzheng-Li <Ethan.Lee.QNL@gmail.com>
> Signed-off-by: Hou Weiying <weiying_hou@outlook.com>
> Signed-off-by: Myriad-Dreamin <camiyoru@gmail.com>
> ---
> target/riscv/pmp.c | 134 ++++++++++++++++++++++++++++++++++----
> target/riscv/pmp.h | 12 ++++
> target/riscv/trace-events | 4 ++
> 3 files changed, 138 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a2b9f5363..b1fa703aff 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -34,6 +34,26 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
> static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
> static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index);
>
> +static char mode_to_char(int mode)
> +{
> + char ret = 0;
> + switch (mode) {
> + case PRV_U:
> + ret = 'u';
> + break;
> + case PRV_S:
> + ret = 's';
> + break;
> + case PRV_H:
> + ret = 'h';
> + break;
> + case PRV_M:
> + ret = 'm';
> + break;
> + }
> + return ret;
> +}
> +
> /*
> * Accessor method to extract address matching type 'a field' from cfg reg
> */
> @@ -99,7 +119,28 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t pmp_index)
> static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
> {
> if (pmp_index < MAX_RISCV_PMPS) {
> - if (!pmp_is_locked(env, pmp_index)) {
> + /*
> + * mseccfg.RLB is set
> + */
> + if (MSECCFG_RLB_ISSET(env) ||
> + /*
> + * mseccfg.MML is set
> + */
> + (MSECCFG_MML_ISSET(env) &&
> + /*
> + * m model and not adding X bit
> + */
> + (((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) ||
> + /*
> + * shared region and not adding X bit
> + */
> + ((val & PMP_LOCK) != PMP_LOCK &&
> + (val & 0x7) != (PMP_WRITE | PMP_EXEC)))) ||
> + /*
> + * mseccfg.MML is not set
> + */
> + (!MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index))
> + ){
> env->pmp_state.pmp[pmp_index].cfg_reg = val;
> pmp_update_rule(env, pmp_index);
> } else {
> @@ -230,6 +271,18 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>
> /* Short cut if no rules */
> if (0 == pmp_get_num_rules(env)) {
> + if (MSECCFG_MMWP_ISSET(env)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> + if (MSECCFG_MML_ISSET(env) && (mode != PRV_M || (privs & PMP_EXEC))) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + return false;
> + }
> return true;
> }
>
> @@ -265,16 +318,65 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> const uint8_t a_field =
> pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
>
> - /*
> - * If the PMP entry is not off and the address is in range, do the priv
> - * check
> - */
> if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
> - allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> - if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> - allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + /*
> + * If the PMP entry is not off and the address is in range,
> + * do the priv check
> + */
> + if (!MSECCFG_MML_ISSET(env)) {
> + /*
> + * If mseccfg.MML Bit is not set, do pmp priv check
> + */
> + allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> + if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> + allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + }
> + } else {
> + /*
> + * If mseccfg.MML Bit set, do the enhanced pmp priv check
> + */
> + if (env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_EXEC | ((mode == PRV_M &&
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_READ : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> +
> + if (mode != PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - %c mode access denied\n",
> + mode_to_char(mode));
> + ret = 0;
> + break;
> + }
> + }
> + } else {
> + /*
> + * Shared Region
> + */
> + if ((env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
> + allowed_privs = PMP_READ | ((mode == PRV_M ||
> + (env->pmp_state.pmp[i].cfg_reg & PMP_EXEC)) ?
> + PMP_WRITE : 0);
> + } else {
> + allowed_privs = env->pmp_state.pmp[i].cfg_reg &
> + (PMP_READ | PMP_WRITE | PMP_EXEC);
> + if (mode == PRV_M && allowed_privs) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "pmp violation - m mode access denied\n");
> + ret = 0;
> + break;
This is really hard to follow, I had a lot of trouble wrapping my head
around this and I'm still not sure it's correct.
I feel that a switch satement here would be easier. The spec has a
nice table of all the possible options, for example something like
this:
if (mode == PRV_M) {
switch (epmp_operation) {
case 0:
case 1:
case 4:
case 5:
case 6:
case 7:
case 8:
allowed_privs = 0;
break;
case 2:
case 3:
case 14:
allowed_privs = PMP_READ | PMP_WRITE;
break;
case 9:
case 10:
allowed_privs = PMP_EXEC;
break;
case 11:
case 13:
allowed_privs = PMP_READ | PMP_EXEC;
break;
case 12:
case 15:
allowed_privs = PMP_READ;
break;
}
} else {
switch (epmp_operation) {
case 0:
case 8:
case 9:
case 12:
case 13:
case 14:
allowed_privs = 0;
break;
case 1:
case 10:
case 11:
allowed_privs = PMP_EXEC;
break;
case 2:
case 4:
case 15:
allowed_privs = PMP_READ;
break;
case 3:
case 6:
allowed_privs = PMP_READ | PMP_WRITE;
break;
case 5:
allowed_privs = PMP_READ | PMP_EXEC;
break;
case 7:
allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
break;
}
}
It's a little more verbose, but it should still be fairly quick to compute.
If you want to keep the logic that's also fine, I just think it needs
some simplifiation. Maybe assign read, write, exec variables and just
compare them?
bool read = env->pmp_state.pmp[i].cfg_reg & PMP_READ;
bool write = env->pmp_state.pmp[i].cfg_reg & PMP_WRITE;
bool exec = env->pmp_state.pmp[i].cfg_reg & PMP_EXEC;
Then
if ((env->pmp_state.pmp[i].cfg_reg & (PMP_READ | PMP_WRITE)) == PMP_WRITE) {
becomes
if (write && !read) {
Alistair
> + }
> + }
> + }
> }
> -
> if ((privs & allowed_privs) == privs) {
> ret = 1;
> break;
> @@ -288,15 +390,23 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> /* No rule matched */
> if (ret == -1) {
> if (mode == PRV_M) {
> - ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
> - * M-Mode access, the access succeeds */
> + ret = !MSECCFG_MMWP_ISSET(env); /* PMP Enhancements */
> + if (MSECCFG_MML_ISSET(env) && (privs & PMP_EXEC)) {
> + ret = 0;
> + }
> } else {
> ret = 0; /* Other modes are not allowed to succeed if they don't
> * match a rule, but there are rules. We've checked for
> * no rule earlier in this function. */
> }
> }
> -
> + if (ret) {
> + trace_pmp_hart_has_privs_pass_match(
> + env->mhartid, addr, size, privs, mode);
> + } else {
> + trace_pmp_hart_has_privs_violation(
> + env->mhartid, addr, size, privs, mode);
> + }
> return ret == 1 ? true : false;
> }
>
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index 8e19793132..7db2069204 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -36,6 +36,12 @@ typedef enum {
> PMP_AMATCH_NAPOT /* Naturally aligned power-of-two region */
> } pmp_am_t;
>
> +typedef enum {
> + MSECCFG_MML = 1 << 0,
> + MSECCFG_MMWP = 1 << 1,
> + MSECCFG_RLB = 1 << 2
> +} mseccfg_field_t;
> +
> typedef struct {
> target_ulong addr_reg;
> uint8_t cfg_reg;
> @@ -58,7 +64,13 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
> void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
> target_ulong val);
> target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
> +void mseccfg_csr_write(CPURISCVState *env, target_ulong val);
> +target_ulong mseccfg_csr_read(CPURISCVState *env);
> bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> target_ulong size, pmp_priv_t priv, target_ulong mode);
>
> +#define MSECCFG_MML_ISSET(env) get_field(env->mseccfg, MSECCFG_MML)
> +#define MSECCFG_MMWP_ISSET(env) get_field(env->mseccfg, MSECCFG_MMWP)
> +#define MSECCFG_RLB_ISSET(env) get_field(env->mseccfg, MSECCFG_RLB)
> +
> #endif
> diff --git a/target/riscv/trace-events b/target/riscv/trace-events
> index 4b6c652ae9..4f877f90f7 100644
> --- a/target/riscv/trace-events
> +++ b/target/riscv/trace-events
> @@ -6,3 +6,7 @@ pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRI
> pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_read(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": read addr%" PRIu32", val: 0x%" PRIx64
> pmpaddr_csr_write(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart %" PRIu64 ": write addr%" PRIu32", val: 0x%" PRIx64
> +mseccfg_csr_read(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": read mseccfg, val: 0x%" PRIx64
> +mseccfg_csr_write(uint64_t mhartid, uint64_t val) "hart %" PRIu64 ": write mseccfg, val: 0x%" PRIx64
> +pmp_hart_has_privs_pass_match(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> +pmp_hart_has_privs_violation(uint64_t mhartid, uint64_t addr, uint64_t size, uint64_t privs, uint64_t mode) "hart %"PRId64 "pass PMP 0 match addr:%"PRIu64" size:%"PRIu64 "privs: %"PRIu64 "mode: %"PRIu64
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-02-10 20:20 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200811002325.46056-1-weiying_hou@outlook.com>
2020-08-11 0:23 ` [PATCH v2 1/4] Define ePMP mseccfg Hou Weiying
2020-08-11 0:23 ` Hou Weiying
2020-08-25 22:31 ` Alistair Francis
2020-08-25 22:31 ` Alistair Francis
2020-08-11 0:23 ` [PATCH v2 2/4] Implementation of enhanced PMP(ePMP) support Hou Weiying
2020-08-11 0:23 ` Hou Weiying
2021-02-09 18:25 ` Alistair Francis
2021-02-09 18:25 ` Alistair Francis
2021-02-10 20:18 ` Alistair Francis
2021-02-10 20:18 ` Alistair Francis
2020-08-11 0:23 ` [PATCH v2 3/4] Add ePMP CSR accesses Hou Weiying
2020-08-11 0:23 ` Hou Weiying
2021-02-10 20:11 ` Alistair Francis
2021-02-10 20:11 ` Alistair Francis
2020-08-11 0:23 ` [PATCH v2 4/4] Add a config option for ePMP Hou Weiying
2020-08-11 0:23 ` Hou Weiying
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.