[Qemu-devel] [PATCH] target/riscv: Do not allow sfence.vma from user mode

Jonathan Behrens posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/CANnJOVHadZt5Vho5sy6FGVgUW_yvYZg7BGDiGJFtRBCquyOXZA@mail.gmail.com
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Palmer Dabbelt <palmer@sifive.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
target/riscv/op_helper.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] target/riscv: Do not allow sfence.vma from user mode
Posted by Jonathan Behrens 5 years ago
The 'sfence.vma' instruction is privileged, and should only ever be allowed
when executing in supervisor mode or higher.

Jonathan

Signed-off-by: Jonathan Behrens <fintelia@gmail.com>
---
 target/riscv/op_helper.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index b7dc18a41e..644d0fb35f 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -145,9 +145,10 @@ void helper_tlb_flush(CPURISCVState *env)
 {
     RISCVCPU *cpu = riscv_env_get_cpu(env);
     CPUState *cs = CPU(cpu);
-    if (env->priv == PRV_S &&
-        env->priv_ver >= PRIV_VERSION_1_10_0 &&
-        get_field(env->mstatus, MSTATUS_TVM)) {
+    if (!(env->priv >= PRV_S) ||
+        (env->priv == PRV_S &&
+         env->priv_ver >= PRIV_VERSION_1_10_0 &&
+         get_field(env->mstatus, MSTATUS_TVM))) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
     } else {
         tlb_flush(cs);
-- 
2.20.1
Re: [Qemu-devel] [PATCH] target/riscv: Do not allow sfence.vma from user mode
Posted by Richard Henderson 5 years ago
On 4/2/19 2:12 AM, Jonathan Behrens wrote:
> The 'sfence.vma' instruction is privileged, and should only ever be allowed
> when executing in supervisor mode or higher.
> 
> Jonathan
> 
> Signed-off-by: Jonathan Behrens <fintelia@gmail.com>
> ---
>  target/riscv/op_helper.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH] target/riscv: Do not allow sfence.vma from user mode
Posted by Alistair Francis 5 years ago
On Mon, Apr 1, 2019 at 1:39 PM Jonathan Behrens <fintelia@gmail.com> wrote:
>
> The 'sfence.vma' instruction is privileged, and should only ever be allowed
> when executing in supervisor mode or higher.
>
> Jonathan
>
> Signed-off-by: Jonathan Behrens <fintelia@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/op_helper.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index b7dc18a41e..644d0fb35f 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -145,9 +145,10 @@ void helper_tlb_flush(CPURISCVState *env)
>  {
>      RISCVCPU *cpu = riscv_env_get_cpu(env);
>      CPUState *cs = CPU(cpu);
> -    if (env->priv == PRV_S &&
> -        env->priv_ver >= PRIV_VERSION_1_10_0 &&
> -        get_field(env->mstatus, MSTATUS_TVM)) {
> +    if (!(env->priv >= PRV_S) ||
> +        (env->priv == PRV_S &&
> +         env->priv_ver >= PRIV_VERSION_1_10_0 &&
> +         get_field(env->mstatus, MSTATUS_TVM))) {
>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>      } else {
>          tlb_flush(cs);
> --
> 2.20.1

Re: [Qemu-devel] [PATCH] target/riscv: Do not allow sfence.vma from user mode
Posted by Jonathan Behrens 5 years ago
Just to double check, nothing further on this is need from me, right? It is
set to be merged onto the master branch once the 4.0 release is out?

Jonathan

On Wed, Apr 3, 2019 at 7:11 PM Alistair Francis <alistair23@gmail.com>
wrote:

> On Mon, Apr 1, 2019 at 1:39 PM Jonathan Behrens <fintelia@gmail.com>
> wrote:
> >
> > The 'sfence.vma' instruction is privileged, and should only ever be
> allowed
> > when executing in supervisor mode or higher.
> >
> > Jonathan
> >
> > Signed-off-by: Jonathan Behrens <fintelia@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
>
> > ---
> >  target/riscv/op_helper.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index b7dc18a41e..644d0fb35f 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -145,9 +145,10 @@ void helper_tlb_flush(CPURISCVState *env)
> >  {
> >      RISCVCPU *cpu = riscv_env_get_cpu(env);
> >      CPUState *cs = CPU(cpu);
> > -    if (env->priv == PRV_S &&
> > -        env->priv_ver >= PRIV_VERSION_1_10_0 &&
> > -        get_field(env->mstatus, MSTATUS_TVM)) {
> > +    if (!(env->priv >= PRV_S) ||
> > +        (env->priv == PRV_S &&
> > +         env->priv_ver >= PRIV_VERSION_1_10_0 &&
> > +         get_field(env->mstatus, MSTATUS_TVM))) {
> >          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> >      } else {
> >          tlb_flush(cs);
> > --
> > 2.20.1
>
Re: [Qemu-devel] [PATCH] target/riscv: Do not allow sfence.vma from user mode
Posted by Alistair Francis 5 years ago
On Fri, Apr 12, 2019 at 2:15 PM Jonathan Behrens <fintelia@gmail.com> wrote:
>
> Just to double check, nothing further on this is need from me, right? It is set to be merged onto the master branch once the 4.0 release is out?

Thanks for checking!

Yep you don't need to do anything, Palmer will merge it in the next
RISC-V PR after 4.0.

Alistair

>
> Jonathan
>
> On Wed, Apr 3, 2019 at 7:11 PM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Mon, Apr 1, 2019 at 1:39 PM Jonathan Behrens <fintelia@gmail.com> wrote:
>> >
>> > The 'sfence.vma' instruction is privileged, and should only ever be allowed
>> > when executing in supervisor mode or higher.
>> >
>> > Jonathan
>> >
>> > Signed-off-by: Jonathan Behrens <fintelia@gmail.com>
>>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>
>> Alistair
>>
>> > ---
>> >  target/riscv/op_helper.c | 7 ++++---
>> >  1 file changed, 4 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>> > index b7dc18a41e..644d0fb35f 100644
>> > --- a/target/riscv/op_helper.c
>> > +++ b/target/riscv/op_helper.c
>> > @@ -145,9 +145,10 @@ void helper_tlb_flush(CPURISCVState *env)
>> >  {
>> >      RISCVCPU *cpu = riscv_env_get_cpu(env);
>> >      CPUState *cs = CPU(cpu);
>> > -    if (env->priv == PRV_S &&
>> > -        env->priv_ver >= PRIV_VERSION_1_10_0 &&
>> > -        get_field(env->mstatus, MSTATUS_TVM)) {
>> > +    if (!(env->priv >= PRV_S) ||
>> > +        (env->priv == PRV_S &&
>> > +         env->priv_ver >= PRIV_VERSION_1_10_0 &&
>> > +         get_field(env->mstatus, MSTATUS_TVM))) {
>> >          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>> >      } else {
>> >          tlb_flush(cs);
>> > --
>> > 2.20.1

Re: [Qemu-devel] [PATCH] target/riscv: Do not allow sfence.vma from user mode
Posted by Palmer Dabbelt 5 years ago
I actually missed this.  I just added it to for-next on 
github.com/palmer-dabbelt.

Thanks for the ping!

On Fri, 12 Apr 2019 14:23:42 PDT (-0700), alistair23@gmail.com wrote:
> On Fri, Apr 12, 2019 at 2:15 PM Jonathan Behrens <fintelia@gmail.com> wrote:
>>
>> Just to double check, nothing further on this is need from me, right? It is set to be merged onto the master branch once the 4.0 release is out?
>
> Thanks for checking!
>
> Yep you don't need to do anything, Palmer will merge it in the next
> RISC-V PR after 4.0.
>
> Alistair
>
>>
>> Jonathan
>>
>> On Wed, Apr 3, 2019 at 7:11 PM Alistair Francis <alistair23@gmail.com> wrote:
>>>
>>> On Mon, Apr 1, 2019 at 1:39 PM Jonathan Behrens <fintelia@gmail.com> wrote:
>>> >
>>> > The 'sfence.vma' instruction is privileged, and should only ever be allowed
>>> > when executing in supervisor mode or higher.
>>> >
>>> > Jonathan
>>> >
>>> > Signed-off-by: Jonathan Behrens <fintelia@gmail.com>
>>>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>
>>> Alistair
>>>
>>> > ---
>>> >  target/riscv/op_helper.c | 7 ++++---
>>> >  1 file changed, 4 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>>> > index b7dc18a41e..644d0fb35f 100644
>>> > --- a/target/riscv/op_helper.c
>>> > +++ b/target/riscv/op_helper.c
>>> > @@ -145,9 +145,10 @@ void helper_tlb_flush(CPURISCVState *env)
>>> >  {
>>> >      RISCVCPU *cpu = riscv_env_get_cpu(env);
>>> >      CPUState *cs = CPU(cpu);
>>> > -    if (env->priv == PRV_S &&
>>> > -        env->priv_ver >= PRIV_VERSION_1_10_0 &&
>>> > -        get_field(env->mstatus, MSTATUS_TVM)) {
>>> > +    if (!(env->priv >= PRV_S) ||
>>> > +        (env->priv == PRV_S &&
>>> > +         env->priv_ver >= PRIV_VERSION_1_10_0 &&
>>> > +         get_field(env->mstatus, MSTATUS_TVM))) {
>>> >          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>>> >      } else {
>>> >          tlb_flush(cs);
>>> > --
>>> > 2.20.1