* RE: [PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0
@ 2023-10-06 1:12 Chang Alvin
0 siblings, 0 replies; 4+ messages in thread
From: Chang Alvin @ 2023-10-06 1:12 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-riscv, qemu-devel, alistair.francis, Andrew Jones, Mayuresh Chitale
[-- Attachment #1: Type: text/plain, Size: 5123 bytes --]
> 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
> >
> >
[-- Attachment #2: Type: text/html, Size: 23704 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0
@ 2023-10-24 0:55 Chang Alvin
0 siblings, 0 replies; 4+ messages in thread
From: Chang Alvin @ 2023-10-24 0:55 UTC (permalink / raw)
To: Alistair Francis; +Cc: qemu-riscv, qemu-devel, alistair.francis
[-- Attachment #1: Type: text/plain, Size: 5002 bytes --]
> 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,
I saw patches from Mayuresh were merged into riscv-to-apply.next yesterday.
Would you also take a look at this patch?
Thanks!
Alvin Chang
> >
> > 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
> >
> >
[-- Attachment #2: Type: text/html, Size: 19236 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0
2023-09-24 16:10 Alvin Chang
@ 2023-09-25 1:52 ` Alistair Francis
0 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2023-09-25 1:52 UTC (permalink / raw)
To: Alvin Chang; +Cc: qemu-riscv, qemu-devel, alistair.francis, ajones
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
>
> 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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0
@ 2023-09-24 16:10 Alvin Chang
2023-09-25 1:52 ` Alistair Francis
0 siblings, 1 reply; 4+ messages in thread
From: Alvin Chang @ 2023-09-24 16:10 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: alistair.francis, ajones, Alvin Chang
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".
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-24 0:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-06 1:12 [PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0 Chang Alvin
-- strict thread matches above, loose matches on Subject: below --
2023-10-24 0:55 Chang Alvin
2023-09-24 16:10 Alvin Chang
2023-09-25 1:52 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.