|
From: | Chang Alvin |
Subject: | RE: [PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0 |
Date: | Fri, 6 Oct 2023 09:12:37 +0800 |
> On Mon, Sep 25, 2023 at 2:11 AM 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 v3: Modify "epmp_operation" to "smepmp_operation".
>
> This has the same issue as all the previous versions.
>
> QEMU is currently not shipping with Smepmp support. So updating some of the
> code to support Smepmp is confusing.
>
> As I pointed out for the v3, we currently only support ePMP 0.9.3. So that is
> what the code must work with.
>
> In order for this change to go in, we also need to upgrade QEMU to support
> Smepmp 1.0.
>
> This patch is an attempt to do that:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg967676.html
>
> You basically need to combine the changes from
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg967676.html into
> this patch. So there is a single patch that updates to Smepmp.
>
> Alistair
>
Hi Alistair,
Mayuresh has sent that patch again recently:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg991428.html
Since the author is from Ventana, I would like to send my patch separately.
I think our patches can be merged/applied together.
Thanks!
Alvin
> >
> > Changes from v2: Adopt switch case ranges and numerical order.
> >
> > Changes from v1: Convert ePMP over to Smepmp.
> >
> > 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..2ebf18c941 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
> > + * Smepmp spec.
> > + */
> > + const uint8_t smepmp_operation =
> > + ((val & PMP_LOCK) >> 4) | ((val & PMP_READ) <<
> 2) |
> > + (val & PMP_WRITE) | ((val & PMP_EXEC) >> 2);
> > +
> > + switch (smepmp_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
> >
> >
[Prev in Thread] | Current Thread | [Next in Thread] |