All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
@ 2020-01-21  5:36 ` Alistair Francis
  0 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-01-21  5:36 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, palmer, alistair23

As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
correctly handling illegal instructions based on the value of MSTATUS_TSR
and the current privledge level.

This patch fixes the issue raised in the bug by raising an illegal
instruction if TSR is set and we are in S-Mode.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/op_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 331cc36232..eed8eea6f2 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
     }
 
     if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
-        get_field(env->mstatus, MSTATUS_TSR)) {
+        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
     }
 
-- 
2.24.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
@ 2020-01-21  5:36 ` Alistair Francis
  0 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-01-21  5:36 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: palmer, alistair.francis, alistair23

As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
correctly handling illegal instructions based on the value of MSTATUS_TSR
and the current privledge level.

This patch fixes the issue raised in the bug by raising an illegal
instruction if TSR is set and we are in S-Mode.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/op_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 331cc36232..eed8eea6f2 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
     }
 
     if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
-        get_field(env->mstatus, MSTATUS_TSR)) {
+        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
     }
 
-- 
2.24.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
  2020-01-21  5:36 ` Alistair Francis
@ 2020-01-21 13:18   ` Jonathan Behrens
  -1 siblings, 0 replies; 8+ messages in thread
From: Jonathan Behrens @ 2020-01-21 13:18 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

Looks good to me. Though this is I think the third bug in privilege
checking in op_helper.c which is only like 150 lines long total. It would
be really good to fully double check that there aren't any more lurking
there...

Reviewed-by: Jonathan Behrens <jonathan@fintelia.io
<palmerdabbelt@google.com>>

On Tue, Jan 21, 2020 at 12:45 AM Alistair Francis <alistair.francis@wdc.com>
wrote:

> As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
> correctly handling illegal instructions based on the value of MSTATUS_TSR
> and the current privledge level.
>
> This patch fixes the issue raised in the bug by raising an illegal
> instruction if TSR is set and we are in S-Mode.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/op_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 331cc36232..eed8eea6f2 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env,
> target_ulong cpu_pc_deb)
>      }
>
>      if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
> -        get_field(env->mstatus, MSTATUS_TSR)) {
> +        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>      }
>
> --
> 2.24.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 2130 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
@ 2020-01-21 13:18   ` Jonathan Behrens
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Behrens @ 2020-01-21 13:18 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
	Palmer Dabbelt, Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

Looks good to me. Though this is I think the third bug in privilege
checking in op_helper.c which is only like 150 lines long total. It would
be really good to fully double check that there aren't any more lurking
there...

Reviewed-by: Jonathan Behrens <jonathan@fintelia.io
<palmerdabbelt@google.com>>

On Tue, Jan 21, 2020 at 12:45 AM Alistair Francis <alistair.francis@wdc.com>
wrote:

> As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
> correctly handling illegal instructions based on the value of MSTATUS_TSR
> and the current privledge level.
>
> This patch fixes the issue raised in the bug by raising an illegal
> instruction if TSR is set and we are in S-Mode.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/op_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 331cc36232..eed8eea6f2 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env,
> target_ulong cpu_pc_deb)
>      }
>
>      if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
> -        get_field(env->mstatus, MSTATUS_TSR)) {
> +        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>      }
>
> --
> 2.24.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 2130 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
  2020-01-21  5:36 ` Alistair Francis
@ 2020-02-20 18:41   ` Alistair Francis
  -1 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-02-20 18:41 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, open list:RISC-V, qemu-devel@nongnu.org Developers

On Mon, Jan 20, 2020 at 9:43 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
> correctly handling illegal instructions based on the value of MSTATUS_TSR
> and the current privledge level.
>
> This patch fixes the issue raised in the bug by raising an illegal
> instruction if TSR is set and we are in S-Mode.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

@Palmer Dabbelt  Ping!

Alistair

> ---
>  target/riscv/op_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 331cc36232..eed8eea6f2 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
>      }
>
>      if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
> -        get_field(env->mstatus, MSTATUS_TSR)) {
> +        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>      }
>
> --
> 2.24.1
>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
@ 2020-02-20 18:41   ` Alistair Francis
  0 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2020-02-20 18:41 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Palmer Dabbelt

On Mon, Jan 20, 2020 at 9:43 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
> correctly handling illegal instructions based on the value of MSTATUS_TSR
> and the current privledge level.
>
> This patch fixes the issue raised in the bug by raising an illegal
> instruction if TSR is set and we are in S-Mode.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

@Palmer Dabbelt  Ping!

Alistair

> ---
>  target/riscv/op_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 331cc36232..eed8eea6f2 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
>      }
>
>      if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
> -        get_field(env->mstatus, MSTATUS_TSR)) {
> +        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>      }
>
> --
> 2.24.1
>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
  2020-02-20 18:41   ` Alistair Francis
@ 2020-03-05 21:48     ` Palmer Dabbelt
  -1 siblings, 0 replies; 8+ messages in thread
From: Palmer Dabbelt @ 2020-03-05 21:48 UTC (permalink / raw)
  To: alistair23; +Cc: qemu-riscv, Alistair Francis, qemu-devel

On Thu, 20 Feb 2020 10:41:35 PST (-0800), alistair23@gmail.com wrote:
> On Mon, Jan 20, 2020 at 9:43 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
>>
>> As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
>> correctly handling illegal instructions based on the value of MSTATUS_TSR
>> and the current privledge level.
>>
>> This patch fixes the issue raised in the bug by raising an illegal
>> instruction if TSR is set and we are in S-Mode.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>
> @Palmer Dabbelt  Ping!

Sorry, I must have missed this.  It's in the queue (with the reviews as
collected by patchwork).

Thanks!

>
> Alistair
>
>> ---
>>  target/riscv/op_helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>> index 331cc36232..eed8eea6f2 100644
>> --- a/target/riscv/op_helper.c
>> +++ b/target/riscv/op_helper.c
>> @@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
>>      }
>>
>>      if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
>> -        get_field(env->mstatus, MSTATUS_TSR)) {
>> +        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
>>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>>      }
>>
>> --
>> 2.24.1
>>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] target/riscv: Correctly implement TSR trap
@ 2020-03-05 21:48     ` Palmer Dabbelt
  0 siblings, 0 replies; 8+ messages in thread
From: Palmer Dabbelt @ 2020-03-05 21:48 UTC (permalink / raw)
  To: alistair23; +Cc: Alistair Francis, qemu-devel, qemu-riscv

On Thu, 20 Feb 2020 10:41:35 PST (-0800), alistair23@gmail.com wrote:
> On Mon, Jan 20, 2020 at 9:43 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
>>
>> As reported in: https://bugs.launchpad.net/qemu/+bug/1851939 we weren't
>> correctly handling illegal instructions based on the value of MSTATUS_TSR
>> and the current privledge level.
>>
>> This patch fixes the issue raised in the bug by raising an illegal
>> instruction if TSR is set and we are in S-Mode.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>
> @Palmer Dabbelt  Ping!

Sorry, I must have missed this.  It's in the queue (with the reviews as
collected by patchwork).

Thanks!

>
> Alistair
>
>> ---
>>  target/riscv/op_helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>> index 331cc36232..eed8eea6f2 100644
>> --- a/target/riscv/op_helper.c
>> +++ b/target/riscv/op_helper.c
>> @@ -83,7 +83,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
>>      }
>>
>>      if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
>> -        get_field(env->mstatus, MSTATUS_TSR)) {
>> +        get_field(env->mstatus, MSTATUS_TSR) && !(env->priv >= PRV_M)) {
>>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
>>      }
>>
>> --
>> 2.24.1
>>


^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-05 21:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21  5:36 [PATCH v1 1/1] target/riscv: Correctly implement TSR trap Alistair Francis
2020-01-21  5:36 ` Alistair Francis
2020-01-21 13:18 ` Jonathan Behrens
2020-01-21 13:18   ` Jonathan Behrens
2020-02-20 18:41 ` Alistair Francis
2020-02-20 18:41   ` Alistair Francis
2020-03-05 21:48   ` Palmer Dabbelt
2020-03-05 21:48     ` Palmer Dabbelt

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.