qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/riscv: update checks on writing pmpcfg for ePMP to version 1.0
@ 2023-09-04 16:19 Alvin Chang
  2023-09-07  3:11 ` Alistair Francis
  0 siblings, 1 reply; 2+ messages in thread
From: Alvin Chang @ 2023-09-04 16:19 UTC (permalink / raw)
  To: qemu-riscv, qemu-devel; +Cc: alistair.francis, Alvin Chang

Current checks on writing pmpcfg for ePMP follows ePMP version 0.9.1.
However, ePMP 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 ePMP 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 ePMP
truth table, and checks the rules by aforementioned specification
changes.

Signed-off-by: Alvin Chang <alvinga@andestech.com>
---
 target/riscv/pmp.c | 51 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index a08cd95658..c036ca3e70 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -99,16 +99,49 @@ 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) {
-                    locked = false;
-                }
-                /* shared region and not adding X bit */
-                if ((val & PMP_LOCK) != PMP_LOCK &&
-                    (val & 0x7) != (PMP_WRITE | 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) {
+                /* pmpcfg.L = 0. Neither M-mode-only nor locked Shared-Region */
+                case 0:
+                case 1:
+                case 2:
+                case 3:
+                case 4:
+                case 5:
+                case 6:
+                case 7:
+                /* pmpcfg.L = 1 and pmpcfg.X = 0 (but case 10 is not allowed) */
+                case 8:
+                case 12:
+                case 14:
+                /* pmpcfg.LRWX = 1111 */
+                case 15:  /* Read-only locked Shared-Region on all modes */
                     locked = false;
+                    break;
+                /* Other rules which add new code regions are not allowed */
+                case 9:
+                case 10:  /* Execute-only locked Shared-Region on all modes */
+                case 11:
+                case 13:
+                    break;
+                default:
+                    g_assert_not_reached();
                 }
             }
         } else {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] target/riscv: update checks on writing pmpcfg for ePMP to version 1.0
  2023-09-04 16:19 [PATCH] target/riscv: update checks on writing pmpcfg for ePMP to version 1.0 Alvin Chang
@ 2023-09-07  3:11 ` Alistair Francis
  0 siblings, 0 replies; 2+ messages in thread
From: Alistair Francis @ 2023-09-07  3:11 UTC (permalink / raw)
  To: Alvin Chang; +Cc: qemu-riscv, qemu-devel, alistair.francis, Alvin Chang

On Tue, Sep 5, 2023 at 2:30 AM Alvin Chang <vivahavey@gmail.com> wrote:
>
> Current checks on writing pmpcfg for ePMP follows ePMP version 0.9.1.
> However, ePMP 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 ePMP 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 ePMP
> truth table, and checks the rules by aforementioned specification
> changes.
>
> Signed-off-by: Alvin Chang <alvinga@andestech.com>

Thanks for the patch!

As part of this change we should convert ePMP over to Smepmp and drop
the experimental status

Alistair

> ---
>  target/riscv/pmp.c | 51 ++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index a08cd95658..c036ca3e70 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -99,16 +99,49 @@ 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) {
> -                    locked = false;
> -                }
> -                /* shared region and not adding X bit */
> -                if ((val & PMP_LOCK) != PMP_LOCK &&
> -                    (val & 0x7) != (PMP_WRITE | 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) {
> +                /* pmpcfg.L = 0. Neither M-mode-only nor locked Shared-Region */
> +                case 0:
> +                case 1:
> +                case 2:
> +                case 3:
> +                case 4:
> +                case 5:
> +                case 6:
> +                case 7:
> +                /* pmpcfg.L = 1 and pmpcfg.X = 0 (but case 10 is not allowed) */
> +                case 8:
> +                case 12:
> +                case 14:
> +                /* pmpcfg.LRWX = 1111 */
> +                case 15:  /* Read-only locked Shared-Region on all modes */
>                      locked = false;
> +                    break;
> +                /* Other rules which add new code regions are not allowed */
> +                case 9:
> +                case 10:  /* Execute-only locked Shared-Region on all modes */
> +                case 11:
> +                case 13:
> +                    break;
> +                default:
> +                    g_assert_not_reached();
>                  }
>              }
>          } else {
> --
> 2.34.1
>
>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-09-07  3:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-04 16:19 [PATCH] target/riscv: update checks on writing pmpcfg for ePMP to version 1.0 Alvin Chang
2023-09-07  3:11 ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).