qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] target/riscv: update checks on writing pmpcfg for Smepmp


From: Chang Alvin
Subject: Re: [PATCH v3] target/riscv: update checks on writing pmpcfg for Smepmp
Date: Tue, 19 Sep 2023 13:08:45 +0800

 > -----Original Message-----

> From: Alistair Francis <alistair23@gmail.com>

> Sent: Tuesday, September 19, 2023 12:42 PM

> To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>

> Cc: qemu-riscv@nongnu.org; qemu-devel@nongnu.org;

> alistair.francis@wdc.com; ajones@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:32PM Alvin Chang <alvinga@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 <alvinga@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

> >

> >


reply via email to

[Prev in Thread] Current Thread [Next in Thread]