* [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode()
@ 2025-07-14 13:37 Daniel Henrique Barboza
2025-07-14 13:40 ` Richard Henderson
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2025-07-14 13:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
Daniel Henrique Barboza, Richard Henderson
GETPC() should always be called from the top level helper, e.g. the
first helper that is called by the translation code. We stopped doing
that in commit 3157a553ec, and then we introduced problems when
unwinding the exceptions being thrown by helper_mret(), as reported by
[1].
Call GETPC() at the top level helper and pass the value along.
[1] https://gitlab.com/qemu-project/qemu/-/issues/3020
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Fixes: 3157a553ec ("target/riscv: Add Smrnmi mnret instruction")
Closes: https://gitlab.com/qemu-project/qemu/-/issues/3020
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/op_helper.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 15460bf84b..110292e84d 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -355,21 +355,22 @@ target_ulong helper_sret(CPURISCVState *env)
}
static void check_ret_from_m_mode(CPURISCVState *env, target_ulong retpc,
- target_ulong prev_priv)
+ target_ulong prev_priv,
+ uintptr_t ra)
{
if (!(env->priv >= PRV_M)) {
- riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
+ riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
}
if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg,
env->priv_ver,
env->misa_ext) && (retpc & 0x3)) {
- riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
+ riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, ra);
}
if (riscv_cpu_cfg(env)->pmp &&
!pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
- riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, GETPC());
+ riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, ra);
}
}
static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,
@@ -394,8 +395,9 @@ target_ulong helper_mret(CPURISCVState *env)
target_ulong retpc = env->mepc & get_xepc_mask(env);
uint64_t mstatus = env->mstatus;
target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
+ uintptr_t ra = GETPC();
- check_ret_from_m_mode(env, retpc, prev_priv);
+ check_ret_from_m_mode(env, retpc, prev_priv, ra);
target_ulong prev_virt = get_field(env->mstatus, MSTATUS_MPV) &&
(prev_priv != PRV_M);
@@ -443,8 +445,9 @@ target_ulong helper_mnret(CPURISCVState *env)
target_ulong retpc = env->mnepc;
target_ulong prev_priv = get_field(env->mnstatus, MNSTATUS_MNPP);
target_ulong prev_virt;
+ uintptr_t ra = GETPC();
- check_ret_from_m_mode(env, retpc, prev_priv);
+ check_ret_from_m_mode(env, retpc, prev_priv, ra);
prev_virt = get_field(env->mnstatus, MNSTATUS_MNPV) &&
(prev_priv != PRV_M);
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode()
2025-07-14 13:37 [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode() Daniel Henrique Barboza
@ 2025-07-14 13:40 ` Richard Henderson
2025-07-14 13:42 ` Richard Henderson
2025-07-14 19:42 ` Philippe Mathieu-Daudé
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2025-07-14 13:40 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer
On 7/14/25 07:37, Daniel Henrique Barboza wrote:
> GETPC() should always be called from the top level helper, e.g. the
> first helper that is called by the translation code. We stopped doing
> that in commit 3157a553ec, and then we introduced problems when
> unwinding the exceptions being thrown by helper_mret(), as reported by
> [1].
>
> Call GETPC() at the top level helper and pass the value along.
>
> [1]https://gitlab.com/qemu-project/qemu/-/issues/3020
>
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> Fixes: 3157a553ec ("target/riscv: Add Smrnmi mnret instruction")
> Closes:https://gitlab.com/qemu-project/qemu/-/issues/3020
> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
> ---
> target/riscv/op_helper.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode()
2025-07-14 13:40 ` Richard Henderson
@ 2025-07-14 13:42 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2025-07-14 13:42 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
qemu-stable
On 7/14/25 07:40, Richard Henderson wrote:
> On 7/14/25 07:37, Daniel Henrique Barboza wrote:
>> GETPC() should always be called from the top level helper, e.g. the
>> first helper that is called by the translation code. We stopped doing
>> that in commit 3157a553ec, and then we introduced problems when
>> unwinding the exceptions being thrown by helper_mret(), as reported by
>> [1].
>>
>> Call GETPC() at the top level helper and pass the value along.
>>
>> [1]https://gitlab.com/qemu-project/qemu/-/issues/3020
>>
>> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
>> Fixes: 3157a553ec ("target/riscv: Add Smrnmi mnret instruction")
>> Closes:https://gitlab.com/qemu-project/qemu/-/issues/3020
>> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
>> ---
>> target/riscv/op_helper.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-stable@nongnu.org
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode()
2025-07-14 13:37 [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode() Daniel Henrique Barboza
2025-07-14 13:40 ` Richard Henderson
@ 2025-07-14 19:42 ` Philippe Mathieu-Daudé
2025-07-21 7:44 ` Nutty Liu
2025-07-29 3:20 ` Alistair Francis
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-14 19:42 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
Richard Henderson
On 14/7/25 15:37, Daniel Henrique Barboza wrote:
> GETPC() should always be called from the top level helper, e.g. the
> first helper that is called by the translation code. We stopped doing
> that in commit 3157a553ec, and then we introduced problems when
> unwinding the exceptions being thrown by helper_mret(), as reported by
> [1].
>
> Call GETPC() at the top level helper and pass the value along.
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/3020
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Fixes: 3157a553ec ("target/riscv: Add Smrnmi mnret instruction")
> Closes: https://gitlab.com/qemu-project/qemu/-/issues/3020
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/op_helper.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode()
2025-07-14 13:37 [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode() Daniel Henrique Barboza
2025-07-14 13:40 ` Richard Henderson
2025-07-14 19:42 ` Philippe Mathieu-Daudé
@ 2025-07-21 7:44 ` Nutty Liu
2025-07-29 3:20 ` Alistair Francis
3 siblings, 0 replies; 6+ messages in thread
From: Nutty Liu @ 2025-07-21 7:44 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer,
Richard Henderson
On 7/14/2025 9:37 PM, Daniel Henrique Barboza wrote:
> GETPC() should always be called from the top level helper, e.g. the
> first helper that is called by the translation code. We stopped doing
> that in commit 3157a553ec, and then we introduced problems when
> unwinding the exceptions being thrown by helper_mret(), as reported by
> [1].
>
> Call GETPC() at the top level helper and pass the value along.
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/3020
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Fixes: 3157a553ec ("target/riscv: Add Smrnmi mnret instruction")
> Closes: https://gitlab.com/qemu-project/qemu/-/issues/3020
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/op_helper.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Nutty Liu <liujingqi@lanxincomputing.com>
Thanks,
Nutty
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 15460bf84b..110292e84d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -355,21 +355,22 @@ target_ulong helper_sret(CPURISCVState *env)
> }
>
> static void check_ret_from_m_mode(CPURISCVState *env, target_ulong retpc,
> - target_ulong prev_priv)
> + target_ulong prev_priv,
> + uintptr_t ra)
> {
> if (!(env->priv >= PRV_M)) {
> - riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
> }
>
> if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg,
> env->priv_ver,
> env->misa_ext) && (retpc & 0x3)) {
> - riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
> + riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, ra);
> }
>
> if (riscv_cpu_cfg(env)->pmp &&
> !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> - riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, GETPC());
> + riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, ra);
> }
> }
> static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,
> @@ -394,8 +395,9 @@ target_ulong helper_mret(CPURISCVState *env)
> target_ulong retpc = env->mepc & get_xepc_mask(env);
> uint64_t mstatus = env->mstatus;
> target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
> + uintptr_t ra = GETPC();
>
> - check_ret_from_m_mode(env, retpc, prev_priv);
> + check_ret_from_m_mode(env, retpc, prev_priv, ra);
>
> target_ulong prev_virt = get_field(env->mstatus, MSTATUS_MPV) &&
> (prev_priv != PRV_M);
> @@ -443,8 +445,9 @@ target_ulong helper_mnret(CPURISCVState *env)
> target_ulong retpc = env->mnepc;
> target_ulong prev_priv = get_field(env->mnstatus, MNSTATUS_MNPP);
> target_ulong prev_virt;
> + uintptr_t ra = GETPC();
>
> - check_ret_from_m_mode(env, retpc, prev_priv);
> + check_ret_from_m_mode(env, retpc, prev_priv, ra);
>
> prev_virt = get_field(env->mnstatus, MNSTATUS_MNPV) &&
> (prev_priv != PRV_M);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode()
2025-07-14 13:37 [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode() Daniel Henrique Barboza
` (2 preceding siblings ...)
2025-07-21 7:44 ` Nutty Liu
@ 2025-07-29 3:20 ` Alistair Francis
3 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2025-07-29 3:20 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
palmer, Richard Henderson
On Tue, Jul 15, 2025 at 1:05 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> GETPC() should always be called from the top level helper, e.g. the
> first helper that is called by the translation code. We stopped doing
> that in commit 3157a553ec, and then we introduced problems when
> unwinding the exceptions being thrown by helper_mret(), as reported by
> [1].
>
> Call GETPC() at the top level helper and pass the value along.
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/3020
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Fixes: 3157a553ec ("target/riscv: Add Smrnmi mnret instruction")
> Closes: https://gitlab.com/qemu-project/qemu/-/issues/3020
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/op_helper.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 15460bf84b..110292e84d 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -355,21 +355,22 @@ target_ulong helper_sret(CPURISCVState *env)
> }
>
> static void check_ret_from_m_mode(CPURISCVState *env, target_ulong retpc,
> - target_ulong prev_priv)
> + target_ulong prev_priv,
> + uintptr_t ra)
> {
> if (!(env->priv >= PRV_M)) {
> - riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
> }
>
> if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg,
> env->priv_ver,
> env->misa_ext) && (retpc & 0x3)) {
> - riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
> + riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, ra);
> }
>
> if (riscv_cpu_cfg(env)->pmp &&
> !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> - riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, GETPC());
> + riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, ra);
> }
> }
> static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,
> @@ -394,8 +395,9 @@ target_ulong helper_mret(CPURISCVState *env)
> target_ulong retpc = env->mepc & get_xepc_mask(env);
> uint64_t mstatus = env->mstatus;
> target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
> + uintptr_t ra = GETPC();
>
> - check_ret_from_m_mode(env, retpc, prev_priv);
> + check_ret_from_m_mode(env, retpc, prev_priv, ra);
>
> target_ulong prev_virt = get_field(env->mstatus, MSTATUS_MPV) &&
> (prev_priv != PRV_M);
> @@ -443,8 +445,9 @@ target_ulong helper_mnret(CPURISCVState *env)
> target_ulong retpc = env->mnepc;
> target_ulong prev_priv = get_field(env->mnstatus, MNSTATUS_MNPP);
> target_ulong prev_virt;
> + uintptr_t ra = GETPC();
>
> - check_ret_from_m_mode(env, retpc, prev_priv);
> + check_ret_from_m_mode(env, retpc, prev_priv, ra);
>
> prev_virt = get_field(env->mnstatus, MNSTATUS_MNPV) &&
> (prev_priv != PRV_M);
> --
> 2.50.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-29 3:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-14 13:37 [PATCH] target/riscv: do not call GETPC() in check_ret_from_m_mode() Daniel Henrique Barboza
2025-07-14 13:40 ` Richard Henderson
2025-07-14 13:42 ` Richard Henderson
2025-07-14 19:42 ` Philippe Mathieu-Daudé
2025-07-21 7:44 ` Nutty Liu
2025-07-29 3:20 ` Alistair Francis
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 NNTP newsgroup(s).