| Message ID | SG2PR02MB263462CCDBCBBAD36983C2CD93450@SG2PR02MB2634.apcprd02.prod.outlook.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
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 > >
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 > >
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