QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] target/riscv: fix wfi exception behavior
@ 2021-04-20 21:36 Jose Martins
  2021-04-27  6:09 ` Alistair Francis
  2021-05-06  2:09 ` Alistair Francis
  0 siblings, 2 replies; 3+ messages in thread
From: Jose Martins @ 2021-04-20 21:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: open list:RISC-V TCG CPUs, Sagar Karandikar, Jose Martins,
	Bastian Koppelmann, Alistair Francis, Palmer Dabbelt

The wfi exception trigger behavior should take into account user mode,
hstatus.vtw, and the fact the an wfi might raise different types of
exceptions depending on various factors:

If supervisor mode is not present:

- an illegal instruction exception should be generated if user mode
executes and wfi instruction and mstatus.tw = 1.

If supervisor mode is present:

- when a wfi instruction is executed, an illegal exception should be triggered
if either the current mode is user or the mode is supervisor and mstatus.tw is
set.

Plus, if the hypervisor extensions are enabled:

- a virtual instruction exception should be raised when a wfi is executed from
virtual-user or virtual-supervisor and hstatus.vtw is set.

Signed-off-by: Jose Martins <josemartins90@gmail.com>
---
Alistair, I hope you've agreed with my argumentis for the previous version      
of the patch. As promised, I submit this version which takes into account M/U 
only harts. It checks for the presence of the RVS extension. If it is         
not present mstatus.tw takes effect over the exection of wfi in user          
mode.                                                                         

 target/riscv/cpu_bits.h  |  1 +
 target/riscv/op_helper.c | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 24b24c69c5..ed8b97c788 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -436,6 +436,7 @@
 #define HSTATUS_HU           0x00000200
 #define HSTATUS_VGEIN        0x0003F000
 #define HSTATUS_VTVM         0x00100000
+#define HSTATUS_VTW          0x00200000
 #define HSTATUS_VTSR         0x00400000
 #if defined(TARGET_RISCV64)
 #define HSTATUS_VSXL        0x300000000
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index d55def76cf..15982a7a33 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -173,10 +173,15 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
 void helper_wfi(CPURISCVState *env)
 {
     CPUState *cs = env_cpu(env);
+    bool rvs = riscv_has_ext(env, RVS);
+    bool prv_u = env->priv == PRV_U;
+    bool prv_s = env->priv == PRV_S;
 
-    if ((env->priv == PRV_S &&
-        get_field(env->mstatus, MSTATUS_TW)) ||
-        riscv_cpu_virt_enabled(env)) {
+    if (((prv_s || (!rvs && prv_u)) && get_field(env->mstatus, MSTATUS_TW)) ||
+        (rvs && prv_u && !riscv_cpu_virt_enabled(env))) {
+        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
+    } else if (riscv_cpu_virt_enabled(env) && (prv_u ||
+        (prv_s && get_field(env->hstatus, HSTATUS_VTW)))) {
         riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
     } else {
         cs->halted = 1;
-- 
2.25.1



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

* Re: [PATCH v2] target/riscv: fix wfi exception behavior
  2021-04-20 21:36 [PATCH v2] target/riscv: fix wfi exception behavior Jose Martins
@ 2021-04-27  6:09 ` Alistair Francis
  2021-05-06  2:09 ` Alistair Francis
  1 sibling, 0 replies; 3+ messages in thread
From: Alistair Francis @ 2021-04-27  6:09 UTC (permalink / raw)
  To: Jose Martins
  Cc: open list:RISC-V TCG CPUs, Sagar Karandikar, Bastian Koppelmann,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

On Wed, Apr 21, 2021 at 7:37 AM Jose Martins <josemartins90@gmail.com> wrote:
>
> The wfi exception trigger behavior should take into account user mode,
> hstatus.vtw, and the fact the an wfi might raise different types of
> exceptions depending on various factors:
>
> If supervisor mode is not present:
>
> - an illegal instruction exception should be generated if user mode
> executes and wfi instruction and mstatus.tw = 1.
>
> If supervisor mode is present:
>
> - when a wfi instruction is executed, an illegal exception should be triggered
> if either the current mode is user or the mode is supervisor and mstatus.tw is
> set.
>
> Plus, if the hypervisor extensions are enabled:
>
> - a virtual instruction exception should be raised when a wfi is executed from
> virtual-user or virtual-supervisor and hstatus.vtw is set.
>
> Signed-off-by: Jose Martins <josemartins90@gmail.com>

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

Alistair

> ---
> Alistair, I hope you've agreed with my argumentis for the previous version
> of the patch. As promised, I submit this version which takes into account M/U
> only harts. It checks for the presence of the RVS extension. If it is
> not present mstatus.tw takes effect over the exection of wfi in user
> mode.
>
>  target/riscv/cpu_bits.h  |  1 +
>  target/riscv/op_helper.c | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 24b24c69c5..ed8b97c788 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -436,6 +436,7 @@
>  #define HSTATUS_HU           0x00000200
>  #define HSTATUS_VGEIN        0x0003F000
>  #define HSTATUS_VTVM         0x00100000
> +#define HSTATUS_VTW          0x00200000
>  #define HSTATUS_VTSR         0x00400000
>  #if defined(TARGET_RISCV64)
>  #define HSTATUS_VSXL        0x300000000
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index d55def76cf..15982a7a33 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -173,10 +173,15 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
>  void helper_wfi(CPURISCVState *env)
>  {
>      CPUState *cs = env_cpu(env);
> +    bool rvs = riscv_has_ext(env, RVS);
> +    bool prv_u = env->priv == PRV_U;
> +    bool prv_s = env->priv == PRV_S;
>
> -    if ((env->priv == PRV_S &&
> -        get_field(env->mstatus, MSTATUS_TW)) ||
> -        riscv_cpu_virt_enabled(env)) {
> +    if (((prv_s || (!rvs && prv_u)) && get_field(env->mstatus, MSTATUS_TW)) ||
> +        (rvs && prv_u && !riscv_cpu_virt_enabled(env))) {
> +        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> +    } else if (riscv_cpu_virt_enabled(env) && (prv_u ||
> +        (prv_s && get_field(env->hstatus, HSTATUS_VTW)))) {
>          riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
>      } else {
>          cs->halted = 1;
> --
> 2.25.1
>
>


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

* Re: [PATCH v2] target/riscv: fix wfi exception behavior
  2021-04-20 21:36 [PATCH v2] target/riscv: fix wfi exception behavior Jose Martins
  2021-04-27  6:09 ` Alistair Francis
@ 2021-05-06  2:09 ` Alistair Francis
  1 sibling, 0 replies; 3+ messages in thread
From: Alistair Francis @ 2021-05-06  2:09 UTC (permalink / raw)
  To: Jose Martins
  Cc: open list:RISC-V TCG CPUs, Sagar Karandikar, Bastian Koppelmann,
	qemu-devel@nongnu.org Developers, Alistair Francis,
	Palmer Dabbelt

On Wed, Apr 21, 2021 at 7:37 AM Jose Martins <josemartins90@gmail.com> wrote:
>
> The wfi exception trigger behavior should take into account user mode,
> hstatus.vtw, and the fact the an wfi might raise different types of
> exceptions depending on various factors:
>
> If supervisor mode is not present:
>
> - an illegal instruction exception should be generated if user mode
> executes and wfi instruction and mstatus.tw = 1.
>
> If supervisor mode is present:
>
> - when a wfi instruction is executed, an illegal exception should be triggered
> if either the current mode is user or the mode is supervisor and mstatus.tw is
> set.
>
> Plus, if the hypervisor extensions are enabled:
>
> - a virtual instruction exception should be raised when a wfi is executed from
> virtual-user or virtual-supervisor and hstatus.vtw is set.
>
> Signed-off-by: Jose Martins <josemartins90@gmail.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> Alistair, I hope you've agreed with my argumentis for the previous version
> of the patch. As promised, I submit this version which takes into account M/U
> only harts. It checks for the presence of the RVS extension. If it is
> not present mstatus.tw takes effect over the exection of wfi in user
> mode.
>
>  target/riscv/cpu_bits.h  |  1 +
>  target/riscv/op_helper.c | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 24b24c69c5..ed8b97c788 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -436,6 +436,7 @@
>  #define HSTATUS_HU           0x00000200
>  #define HSTATUS_VGEIN        0x0003F000
>  #define HSTATUS_VTVM         0x00100000
> +#define HSTATUS_VTW          0x00200000
>  #define HSTATUS_VTSR         0x00400000
>  #if defined(TARGET_RISCV64)
>  #define HSTATUS_VSXL        0x300000000
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index d55def76cf..15982a7a33 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -173,10 +173,15 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
>  void helper_wfi(CPURISCVState *env)
>  {
>      CPUState *cs = env_cpu(env);
> +    bool rvs = riscv_has_ext(env, RVS);
> +    bool prv_u = env->priv == PRV_U;
> +    bool prv_s = env->priv == PRV_S;
>
> -    if ((env->priv == PRV_S &&
> -        get_field(env->mstatus, MSTATUS_TW)) ||
> -        riscv_cpu_virt_enabled(env)) {
> +    if (((prv_s || (!rvs && prv_u)) && get_field(env->mstatus, MSTATUS_TW)) ||
> +        (rvs && prv_u && !riscv_cpu_virt_enabled(env))) {
> +        riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> +    } else if (riscv_cpu_virt_enabled(env) && (prv_u ||
> +        (prv_s && get_field(env->hstatus, HSTATUS_VTW)))) {
>          riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
>      } else {
>          cs->halted = 1;
> --
> 2.25.1
>
>


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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20 21:36 [PATCH v2] target/riscv: fix wfi exception behavior Jose Martins
2021-04-27  6:09 ` Alistair Francis
2021-05-06  2:09 ` Alistair Francis

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git
	git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git