On Fri, Sep 15, 2023 at 6:32 PM Alvin Chang <alvi...@andestech.com> wrote: > > Current checks on writing pmpcfg for Smepmp follows Smepmp version > 0.9.1. However, Smepmp specification has already been ratified, and > there are some differences between version 0.9.1 and 1.0. In this > commit we update the checks of writing pmpcfg to follow Smepmp version > 1.0. > > When mseccfg.MML is set, the constraints to modify PMP rules are: > 1. Locked rules cannot be removed or modified until a PMP reset, unless > mseccfg.RLB is set. > 2. From Smepmp specification version 1.0, chapter 2 section 4b: > Adding a rule with executable privileges that either is M-mode-only > or a locked Shared-Region is not possible and such pmpcfg writes are > ignored, leaving pmpcfg unchanged. > > The commit transfers the value of pmpcfg into the index of the Smepmp > truth table, and checks the rules by aforementioned specification > changes. > > Signed-off-by: Alvin Chang <alvi...@andestech.com> > --- > Changes from v2: Adopt switch case ranges and numerical order. > > Changes from v1: Convert ePMP over to Smepmp.
Did this part get lost? Alistair > > target/riscv/pmp.c | 40 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 32 insertions(+), 8 deletions(-) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index a08cd95658..b144932b3b 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -99,16 +99,40 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t > pmp_index, uint8_t val) > locked = false; > } > > - /* mseccfg.MML is set */ > - if (MSECCFG_MML_ISSET(env)) { > - /* not adding execute bit */ > - if ((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) { > + /* > + * mseccfg.MML is set. Locked rules cannot be removed or modified > + * until a PMP reset. Besides, from Smepmp specification version > 1.0 > + * , chapter 2 section 4b says: > + * Adding a rule with executable privileges that either is > + * M-mode-only or a locked Shared-Region is not possible and such > + * pmpcfg writes are ignored, leaving pmpcfg unchanged. > + */ > + if (MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index)) { > + /* > + * Convert the PMP permissions to match the truth table in > the > + * ePMP spec. > + */ > + const uint8_t epmp_operation = > + ((val & PMP_LOCK) >> 4) | ((val & PMP_READ) << 2) | > + (val & PMP_WRITE) | ((val & PMP_EXEC) >> 2); > + > + switch (epmp_operation) { > + case 0 ... 8: > locked = false; > - } > - /* shared region and not adding X bit */ > - if ((val & PMP_LOCK) != PMP_LOCK && > - (val & 0x7) != (PMP_WRITE | PMP_EXEC)) { > + break; > + case 9 ... 11: > + break; > + case 12: > + locked = false; > + break; > + case 13: > + break; > + case 14: > + case 15: > locked = false; > + break; > + default: > + g_assert_not_reached(); > } > } > } else { > -- > 2.34.1 > >