> -----Original Message-----
> From: Alistair Francis <alistai...@gmail.com>
> Sent: Tuesday, September 19, 2023 12:42 PM
> To: Alvin Che-Chia Chang(張哲嘉) <alvi...@andestech.com>
> Cc: qemu-ri...@nongnu.org; qemu-devel@nongnu.org;
> alistair.fran...@wdc.com; ajo...@ventanamicro.com
> Subject: Re: [PATCH v3] target/riscv: update checks on writing pmpcfg for
> Smepmp to version 1.0
>
> 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
>
Sorry, do you mean that the term "ePMP" should be changed to "Smepmp" in
source code?
Alvin Chang
> >
> > 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
> >
> >
