* [PATCH] target/riscv: Raise exceptions on wrs.nto
@ 2024-03-08 8:12 Andrew Jones
2024-03-08 9:35 ` Christoph Müllner
2024-03-12 15:30 ` Andrew Jones
0 siblings, 2 replies; 3+ messages in thread
From: Andrew Jones @ 2024-03-08 8:12 UTC (permalink / raw)
To: qemu-riscv
Cc: christoph.muellner, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu
Implementing wrs.nto to always just return is consistent with the
specification, as the instruction is permitted to terminate the
stall for any reason, but it's not useful for virtualization, where
we'd like the guest to trap to the hypervisor in order to allow
scheduling of the lock holding VCPU. Change to always immediately
raise exceptions when the appropriate conditions are present,
otherwise continue to just return. Note, immediately raising
exceptions is also consistent with the specification since the
time limit that should expire prior to the exception is
implementation-specific.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
target/riscv/helper.h | 1 +
target/riscv/insn_trans/trans_rvzawrs.c.inc | 27 ++++++++++++++-------
target/riscv/op_helper.c | 11 +++++++++
3 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 8a635238514d..451261ce5a4f 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -132,6 +132,7 @@ DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl)
DEF_HELPER_1(sret, tl, env)
DEF_HELPER_1(mret, tl, env)
DEF_HELPER_1(wfi, void, env)
+DEF_HELPER_1(wrs_nto, void, env)
DEF_HELPER_1(tlb_flush, void, env)
DEF_HELPER_1(tlb_flush_all, void, env)
/* Native Debug */
diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc b/target/riscv/insn_trans/trans_rvzawrs.c.inc
index 32efbff4d5a5..5dfc3a943fde 100644
--- a/target/riscv/insn_trans/trans_rvzawrs.c.inc
+++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
@@ -16,7 +16,7 @@
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
-static bool trans_wrs(DisasContext *ctx)
+static bool trans_wrs_sto(DisasContext *ctx, arg_wrs_sto *a)
{
if (!ctx->cfg_ptr->ext_zawrs) {
return false;
@@ -40,12 +40,21 @@ static bool trans_wrs(DisasContext *ctx)
return true;
}
-#define GEN_TRANS_WRS(insn) \
-static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn *a) \
-{ \
- (void)a; \
- return trans_wrs(ctx); \
-}
+static bool trans_wrs_nto(DisasContext *ctx, arg_wrs_nto *a)
+{
+ if (!ctx->cfg_ptr->ext_zawrs) {
+ return false;
+ }
-GEN_TRANS_WRS(wrs_nto)
-GEN_TRANS_WRS(wrs_sto)
+ /*
+ * Depending on the mode of execution, mstatus.TW and hstatus.VTW, wrs.nto
+ * should raise an exception when the implementation-specific bounded time
+ * limit has expired. Our time limit is zero, so we either return
+ * immediately, as does our implementation of wrs.sto, or raise an
+ * exception, as handled by the wrs.nto helper.
+ */
+ gen_helper_wrs_nto(tcg_env);
+
+ /* We only get here when helper_wrs_nto() doesn't raise an exception. */
+ return trans_wrs_sto(ctx, NULL);
+}
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f414aaebdbab..2baf5bc3ca19 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -380,6 +380,17 @@ void helper_wfi(CPURISCVState *env)
}
}
+void helper_wrs_nto(CPURISCVState *env)
+{
+ if (env->virt_enabled && (env->priv == PRV_S || env->priv == PRV_U) &&
+ get_field(env->hstatus, HSTATUS_VTW) &&
+ !get_field(env->mstatus, MSTATUS_TW)) {
+ riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
+ } else if (env->priv != PRV_M && get_field(env->mstatus, MSTATUS_TW)) {
+ riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
+ }
+}
+
void helper_tlb_flush(CPURISCVState *env)
{
CPUState *cs = env_cpu(env);
--
2.44.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] target/riscv: Raise exceptions on wrs.nto
2024-03-08 8:12 [PATCH] target/riscv: Raise exceptions on wrs.nto Andrew Jones
@ 2024-03-08 9:35 ` Christoph Müllner
2024-03-12 15:30 ` Andrew Jones
1 sibling, 0 replies; 3+ messages in thread
From: Christoph Müllner @ 2024-03-08 9:35 UTC (permalink / raw)
To: Andrew Jones
Cc: qemu-riscv, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu
[-- Attachment #1: Type: text/plain, Size: 4303 bytes --]
On Fri, Mar 8, 2024 at 9:12 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> Implementing wrs.nto to always just return is consistent with the
> specification, as the instruction is permitted to terminate the
> stall for any reason, but it's not useful for virtualization, where
> we'd like the guest to trap to the hypervisor in order to allow
> scheduling of the lock holding VCPU. Change to always immediately
> raise exceptions when the appropriate conditions are present,
> otherwise continue to just return. Note, immediately raising
> exceptions is also consistent with the specification since the
> time limit that should expire prior to the exception is
> implementation-specific.
>
I've reviewed this against the Zawrs spec (triggered exceptions and their
conditions).
Reviewed-by: Christoph Müllner <christoph.muellner@vrull.eu>
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> target/riscv/helper.h | 1 +
> target/riscv/insn_trans/trans_rvzawrs.c.inc | 27 ++++++++++++++-------
> target/riscv/op_helper.c | 11 +++++++++
> 3 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 8a635238514d..451261ce5a4f 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -132,6 +132,7 @@ DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl)
> DEF_HELPER_1(sret, tl, env)
> DEF_HELPER_1(mret, tl, env)
> DEF_HELPER_1(wfi, void, env)
> +DEF_HELPER_1(wrs_nto, void, env)
> DEF_HELPER_1(tlb_flush, void, env)
> DEF_HELPER_1(tlb_flush_all, void, env)
> /* Native Debug */
> diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc
> b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> index 32efbff4d5a5..5dfc3a943fde 100644
> --- a/target/riscv/insn_trans/trans_rvzawrs.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> @@ -16,7 +16,7 @@
> * this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> -static bool trans_wrs(DisasContext *ctx)
> +static bool trans_wrs_sto(DisasContext *ctx, arg_wrs_sto *a)
> {
> if (!ctx->cfg_ptr->ext_zawrs) {
> return false;
> @@ -40,12 +40,21 @@ static bool trans_wrs(DisasContext *ctx)
> return true;
> }
>
> -#define GEN_TRANS_WRS(insn) \
> -static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn *a) \
> -{ \
> - (void)a; \
> - return trans_wrs(ctx); \
> -}
> +static bool trans_wrs_nto(DisasContext *ctx, arg_wrs_nto *a)
> +{
> + if (!ctx->cfg_ptr->ext_zawrs) {
> + return false;
> + }
>
> -GEN_TRANS_WRS(wrs_nto)
> -GEN_TRANS_WRS(wrs_sto)
> + /*
> + * Depending on the mode of execution, mstatus.TW and hstatus.VTW,
> wrs.nto
> + * should raise an exception when the implementation-specific bounded
> time
> + * limit has expired. Our time limit is zero, so we either return
> + * immediately, as does our implementation of wrs.sto, or raise an
> + * exception, as handled by the wrs.nto helper.
> + */
> + gen_helper_wrs_nto(tcg_env);
> +
> + /* We only get here when helper_wrs_nto() doesn't raise an exception.
> */
> + return trans_wrs_sto(ctx, NULL);
> +}
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index f414aaebdbab..2baf5bc3ca19 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -380,6 +380,17 @@ void helper_wfi(CPURISCVState *env)
> }
> }
>
> +void helper_wrs_nto(CPURISCVState *env)
> +{
> + if (env->virt_enabled && (env->priv == PRV_S || env->priv == PRV_U) &&
> + get_field(env->hstatus, HSTATUS_VTW) &&
> + !get_field(env->mstatus, MSTATUS_TW)) {
> + riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT,
> GETPC());
> + } else if (env->priv != PRV_M && get_field(env->mstatus, MSTATUS_TW))
> {
> + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> + }
> +}
> +
> void helper_tlb_flush(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
> --
> 2.44.0
>
>
[-- Attachment #2: Type: text/html, Size: 5485 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] target/riscv: Raise exceptions on wrs.nto
2024-03-08 8:12 [PATCH] target/riscv: Raise exceptions on wrs.nto Andrew Jones
2024-03-08 9:35 ` Christoph Müllner
@ 2024-03-12 15:30 ` Andrew Jones
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Jones @ 2024-03-12 15:30 UTC (permalink / raw)
To: qemu-riscv
Cc: christoph.muellner, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu
On Fri, Mar 08, 2024 at 09:12:20AM +0100, Andrew Jones wrote:
> Implementing wrs.nto to always just return is consistent with the
> specification, as the instruction is permitted to terminate the
> stall for any reason, but it's not useful for virtualization, where
> we'd like the guest to trap to the hypervisor in order to allow
> scheduling of the lock holding VCPU. Change to always immediately
> raise exceptions when the appropriate conditions are present,
> otherwise continue to just return. Note, immediately raising
> exceptions is also consistent with the specification since the
> time limit that should expire prior to the exception is
> implementation-specific.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> target/riscv/helper.h | 1 +
> target/riscv/insn_trans/trans_rvzawrs.c.inc | 27 ++++++++++++++-------
> target/riscv/op_helper.c | 11 +++++++++
> 3 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 8a635238514d..451261ce5a4f 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -132,6 +132,7 @@ DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl)
> DEF_HELPER_1(sret, tl, env)
> DEF_HELPER_1(mret, tl, env)
> DEF_HELPER_1(wfi, void, env)
> +DEF_HELPER_1(wrs_nto, void, env)
This is inside an #ifndef CONFIG_USER_ONLY, so I should have wrapped
the call to it with an #ifndef CONFIG_USER_ONLY as well. I just sent
a v2 doing that.
Thanks,
drew
> DEF_HELPER_1(tlb_flush, void, env)
> DEF_HELPER_1(tlb_flush_all, void, env)
> /* Native Debug */
> diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> index 32efbff4d5a5..5dfc3a943fde 100644
> --- a/target/riscv/insn_trans/trans_rvzawrs.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> @@ -16,7 +16,7 @@
> * this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> -static bool trans_wrs(DisasContext *ctx)
> +static bool trans_wrs_sto(DisasContext *ctx, arg_wrs_sto *a)
> {
> if (!ctx->cfg_ptr->ext_zawrs) {
> return false;
> @@ -40,12 +40,21 @@ static bool trans_wrs(DisasContext *ctx)
> return true;
> }
>
> -#define GEN_TRANS_WRS(insn) \
> -static bool trans_ ## insn(DisasContext *ctx, arg_ ## insn *a) \
> -{ \
> - (void)a; \
> - return trans_wrs(ctx); \
> -}
> +static bool trans_wrs_nto(DisasContext *ctx, arg_wrs_nto *a)
> +{
> + if (!ctx->cfg_ptr->ext_zawrs) {
> + return false;
> + }
>
> -GEN_TRANS_WRS(wrs_nto)
> -GEN_TRANS_WRS(wrs_sto)
> + /*
> + * Depending on the mode of execution, mstatus.TW and hstatus.VTW, wrs.nto
> + * should raise an exception when the implementation-specific bounded time
> + * limit has expired. Our time limit is zero, so we either return
> + * immediately, as does our implementation of wrs.sto, or raise an
> + * exception, as handled by the wrs.nto helper.
> + */
> + gen_helper_wrs_nto(tcg_env);
> +
> + /* We only get here when helper_wrs_nto() doesn't raise an exception. */
> + return trans_wrs_sto(ctx, NULL);
> +}
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index f414aaebdbab..2baf5bc3ca19 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -380,6 +380,17 @@ void helper_wfi(CPURISCVState *env)
> }
> }
>
> +void helper_wrs_nto(CPURISCVState *env)
> +{
> + if (env->virt_enabled && (env->priv == PRV_S || env->priv == PRV_U) &&
> + get_field(env->hstatus, HSTATUS_VTW) &&
> + !get_field(env->mstatus, MSTATUS_TW)) {
> + riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
> + } else if (env->priv != PRV_M && get_field(env->mstatus, MSTATUS_TW)) {
> + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> + }
> +}
> +
> void helper_tlb_flush(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-03-12 15:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-08 8:12 [PATCH] target/riscv: Raise exceptions on wrs.nto Andrew Jones
2024-03-08 9:35 ` Christoph Müllner
2024-03-12 15:30 ` Andrew Jones
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.