* [PATCH] target/riscv/pmp: fix no pmp illegal intrs
@ 2021-12-14 9:26 Nikita Shubin
2021-12-14 21:12 ` Richard Henderson
2021-12-21 6:34 ` Alistair Francis
0 siblings, 2 replies; 7+ messages in thread
From: Nikita Shubin @ 2021-12-14 9:26 UTC (permalink / raw)
Cc: qemu-riscv, Nikita Shubin, Bin Meng, qemu-devel, Alistair Francis,
atishp, Palmer Dabbelt
From: Nikita Shubin <n.shubin@yadro.com>
As per the privilege specification, any access from S/U mode should fail
if no pmp region is configured and pmp is present, othwerwise access
should succeed.
Fixes: d102f19a208 (target/riscv/pmp: Raise exception if no PMP entry is configured)
Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
target/riscv/op_helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index ee7c24efe7..58d992e98a 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -146,7 +146,8 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
uint64_t mstatus = env->mstatus;
target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
- if (!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
+ if (riscv_feature(env, RISCV_FEATURE_PMP) &&
+ !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
}
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] target/riscv/pmp: fix no pmp illegal intrs
2021-12-14 9:26 [PATCH] target/riscv/pmp: fix no pmp illegal intrs Nikita Shubin
@ 2021-12-14 21:12 ` Richard Henderson
2021-12-14 21:13 ` Richard Henderson
2021-12-21 6:34 ` Alistair Francis
1 sibling, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2021-12-14 21:12 UTC (permalink / raw)
To: Nikita Shubin
Cc: qemu-riscv, Nikita Shubin, Bin Meng, qemu-devel, Alistair Francis,
atishp, Palmer Dabbelt
On 12/14/21 1:26 AM, Nikita Shubin wrote:
> - if (!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> + if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> + !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
When would the number of rules become non-zero with PMP disabled?
When does this test make a difference?
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/riscv/pmp: fix no pmp illegal intrs
2021-12-14 21:12 ` Richard Henderson
@ 2021-12-14 21:13 ` Richard Henderson
2021-12-15 7:57 ` Nikita Shubin
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2021-12-14 21:13 UTC (permalink / raw)
To: Nikita Shubin
Cc: qemu-riscv, Nikita Shubin, Bin Meng, qemu-devel, Alistair Francis,
atishp, Palmer Dabbelt
On 12/14/21 1:12 PM, Richard Henderson wrote:
> On 12/14/21 1:26 AM, Nikita Shubin wrote:
>> - if (!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
>> + if (riscv_feature(env, RISCV_FEATURE_PMP) &&
>> + !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
>
> When would the number of rules become non-zero with PMP disabled?
> When does this test make a difference?
Oh, nevermind, I see what you mean.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/riscv/pmp: fix no pmp illegal intrs
2021-12-14 21:13 ` Richard Henderson
@ 2021-12-15 7:57 ` Nikita Shubin
0 siblings, 0 replies; 7+ messages in thread
From: Nikita Shubin @ 2021-12-15 7:57 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-riscv, Nikita Shubin, Bin Meng, qemu-devel, Alistair Francis,
atishp, Palmer Dabbelt
Hello Richard!
On Tue, 14 Dec 2021 13:13:57 -0800
Richard Henderson <richard.henderson@linaro.org> wrote:
> On 12/14/21 1:12 PM, Richard Henderson wrote:
> > On 12/14/21 1:26 AM, Nikita Shubin wrote:
> >> - if (!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> >> + if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> >> + !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> >
> > When would the number of rules become non-zero with PMP disabled?
> > When does this test make a difference?
>
> Oh, nevermind, I see what you mean.
>
Np, let me explain in details:
The ISA states:
> Platforms vary widely in demands for physical memory protection, and
> some platforms may provide other PMP structures in addition to or
> instead of the scheme described in this section.
So we might don't have PMP at all, but if we set
qdev_prop_set_bit(DEVICE(obj), "pmp", false);
for some CPU we still end up in illegal inst on mret, cause we get
pmp_get_num_rules(env) == 0, becouse we have no PMP which leads to zero
available rules.
>
> r~
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/riscv/pmp: fix no pmp illegal intrs
2021-12-14 9:26 [PATCH] target/riscv/pmp: fix no pmp illegal intrs Nikita Shubin
2021-12-14 21:12 ` Richard Henderson
@ 2021-12-21 6:34 ` Alistair Francis
2021-12-21 7:13 ` Nikita Shubin
1 sibling, 1 reply; 7+ messages in thread
From: Alistair Francis @ 2021-12-21 6:34 UTC (permalink / raw)
To: Nikita Shubin
Cc: open list:RISC-V, Nikita Shubin, Bin Meng,
qemu-devel@nongnu.org Developers, Alistair Francis, Atish Patra,
Palmer Dabbelt
On Wed, Dec 15, 2021 at 1:00 AM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> From: Nikita Shubin <n.shubin@yadro.com>
>
> As per the privilege specification, any access from S/U mode should fail
> if no pmp region is configured and pmp is present, othwerwise access
> should succeed.
>
> Fixes: d102f19a208 (target/riscv/pmp: Raise exception if no PMP entry is configured)
> Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
Whoops!
I sent a patch to fix the exact same issue :)
I'll drop mine and we can merge yours. Do you mind adding this and
resending the patch
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/585
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/op_helper.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index ee7c24efe7..58d992e98a 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -146,7 +146,8 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
> uint64_t mstatus = env->mstatus;
> target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
>
> - if (!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> + if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> + !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> }
>
> --
> 2.31.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] target/riscv/pmp: fix no pmp illegal intrs
2021-12-21 6:34 ` Alistair Francis
@ 2021-12-21 7:13 ` Nikita Shubin
2021-12-22 22:15 ` Alistair Francis
0 siblings, 1 reply; 7+ messages in thread
From: Nikita Shubin @ 2021-12-21 7:13 UTC (permalink / raw)
Cc: qemu-riscv, Nikita Shubin, Bin Meng, qemu-devel, Palmer Dabbelt,
Alistair Francis
From: Nikita Shubin <n.shubin@yadro.com>
As per the privilege specification, any access from S/U mode should fail
if no pmp region is configured and pmp is present, othwerwise access
should succeed.
Fixes: d102f19a208 (target/riscv/pmp: Raise exception if no PMP entry is configured)
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/585
Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/op_helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index ee7c24efe7..58d992e98a 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -146,7 +146,8 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
uint64_t mstatus = env->mstatus;
target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
- if (!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
+ if (riscv_feature(env, RISCV_FEATURE_PMP) &&
+ !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
}
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] target/riscv/pmp: fix no pmp illegal intrs
2021-12-21 7:13 ` Nikita Shubin
@ 2021-12-22 22:15 ` Alistair Francis
0 siblings, 0 replies; 7+ messages in thread
From: Alistair Francis @ 2021-12-22 22:15 UTC (permalink / raw)
To: Nikita Shubin
Cc: open list:RISC-V, Nikita Shubin, Bin Meng,
qemu-devel@nongnu.org Developers, Palmer Dabbelt,
Alistair Francis
On Tue, Dec 21, 2021 at 6:08 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> From: Nikita Shubin <n.shubin@yadro.com>
>
> As per the privilege specification, any access from S/U mode should fail
> if no pmp region is configured and pmp is present, othwerwise access
> should succeed.
>
> Fixes: d102f19a208 (target/riscv/pmp: Raise exception if no PMP entry is configured)
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/585
> Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/op_helper.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index ee7c24efe7..58d992e98a 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -146,7 +146,8 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
> uint64_t mstatus = env->mstatus;
> target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
>
> - if (!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> + if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> + !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> }
>
> --
> 2.31.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-22 22:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-14 9:26 [PATCH] target/riscv/pmp: fix no pmp illegal intrs Nikita Shubin
2021-12-14 21:12 ` Richard Henderson
2021-12-14 21:13 ` Richard Henderson
2021-12-15 7:57 ` Nikita Shubin
2021-12-21 6:34 ` Alistair Francis
2021-12-21 7:13 ` Nikita Shubin
2021-12-22 22:15 ` 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).