From: Andrew Jones <ajones@ventanamicro.com>
To: qemu-riscv@nongnu.org
Cc: christoph.muellner@vrull.eu, palmer@dabbelt.com,
alistair.francis@wdc.com, bin.meng@windriver.com,
liwei1518@gmail.com, dbarboza@ventanamicro.com,
zhiwei_liu@linux.alibaba.com
Subject: Re: [PATCH v2] target/riscv: Raise exceptions on wrs.nto
Date: Wed, 24 Apr 2024 15:59:45 +0200 [thread overview]
Message-ID: <20240424-b70cfbbeca1a3870c53afbc2@orel> (raw)
In-Reply-To: <20240312152901.512001-2-ajones@ventanamicro.com>
On Tue, Mar 12, 2024 at 04:29:02PM +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>
> Reviewed-by: Christoph M??llner <christoph.muellner@vrull.eu>
I came here to write a gentle ping and noticed this should be 'Müllner'.
I'm not sure what happened to the utf8.
I'll send a v3 with the tag fixed.
Thanks,
drew
> ---
> v2:
> - Added #ifndef CONFIG_USER_ONLY around helper call
>
> target/riscv/helper.h | 1 +
> target/riscv/insn_trans/trans_rvzawrs.c.inc | 29 ++++++++++++++-------
> target/riscv/op_helper.c | 11 ++++++++
> 3 files changed, 32 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..0eef03383889 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,23 @@ 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.
> + */
> +#ifndef CONFIG_USER_ONLY
> + gen_helper_wrs_nto(tcg_env);
> +#endif
> +
> + /* 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
>
prev parent reply other threads:[~2024-04-24 14:00 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-12 15:29 [PATCH v2] target/riscv: Raise exceptions on wrs.nto Andrew Jones
2024-04-24 13:59 ` Andrew Jones [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240424-b70cfbbeca1a3870c53afbc2@orel \
--to=ajones@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=bin.meng@windriver.com \
--cc=christoph.muellner@vrull.eu \
--cc=dbarboza@ventanamicro.com \
--cc=liwei1518@gmail.com \
--cc=palmer@dabbelt.com \
--cc=qemu-riscv@nongnu.org \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).