If we generate an exception for WFIT we could also fill out the rv/rn
fields of the ISS. To facilitate this pass the register number
directly and read the timeout value in the helper itself.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/tcg/helper-defs.h | 2 +-
target/arm/tcg/op_helper.c | 4 +++-
target/arm/tcg/translate-a64.c | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/arm/tcg/helper-defs.h b/target/arm/tcg/helper-defs.h
index 5a10a9fba3b..a9a062cf777 100644
--- a/target/arm/tcg/helper-defs.h
+++ b/target/arm/tcg/helper-defs.h
@@ -55,7 +55,7 @@ DEF_HELPER_2(exception_pc_alignment, noreturn, env, vaddr)
DEF_HELPER_1(setend, void, env)
DEF_HELPER_2(wfi, void, env, i32)
DEF_HELPER_1(wfe, void, env)
-DEF_HELPER_2(wfit, void, env, i64)
+DEF_HELPER_FLAGS_2(wfit, TCG_CALL_NO_WG, void, env, i32)
DEF_HELPER_1(yield, void, env)
DEF_HELPER_1(pre_hvc, void, env)
DEF_HELPER_2(pre_smc, void, env, i32)
diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index aa14f15eb62..28a1c7243ad 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -409,7 +409,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
#endif
}
-void HELPER(wfit)(CPUARMState *env, uint64_t timeout)
+void HELPER(wfit)(CPUARMState *env, uint32_t rd)
{
#ifdef CONFIG_USER_ONLY
/*
@@ -428,6 +428,8 @@ void HELPER(wfit)(CPUARMState *env, uint64_t timeout)
int target_el = check_wfx_trap(env, false, &excp);
/* The WFIT should time out when CNTVCT_EL0 >= the specified value. */
uint64_t cntval = gt_get_countervalue(env);
+ uint64_t timeout = env->xregs[rd];
+
/*
* We want the value that we would get if we read CNTVCT_EL0 from
* the current exception level, so the direct_access offset, not
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 5d261a5e32b..073454b9195 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2064,7 +2064,7 @@ static bool trans_WFIT(DisasContext *s, arg_WFIT *a)
}
gen_a64_update_pc(s, 4);
- gen_helper_wfit(tcg_env, cpu_reg(s, a->rd));
+ gen_helper_wfit(tcg_env, tcg_constant_i32(a->rd));
/* Go back to the main loop to check for interrupts */
s->base.is_jmp = DISAS_EXIT;
return true;
--
2.47.3
On Tue, 24 Feb 2026 at 15:43, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> If we generate an exception for WFIT we could also fill out the rv/rn
> fields of the ISS. To facilitate this pass the register number
> directly and read the timeout value in the helper itself.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target/arm/tcg/helper-defs.h | 2 +-
> target/arm/tcg/op_helper.c | 4 +++-
> target/arm/tcg/translate-a64.c | 2 +-
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/tcg/helper-defs.h b/target/arm/tcg/helper-defs.h
> index 5a10a9fba3b..a9a062cf777 100644
> --- a/target/arm/tcg/helper-defs.h
> +++ b/target/arm/tcg/helper-defs.h
> @@ -55,7 +55,7 @@ DEF_HELPER_2(exception_pc_alignment, noreturn, env, vaddr)
> DEF_HELPER_1(setend, void, env)
> DEF_HELPER_2(wfi, void, env, i32)
> DEF_HELPER_1(wfe, void, env)
> -DEF_HELPER_2(wfit, void, env, i64)
> +DEF_HELPER_FLAGS_2(wfit, TCG_CALL_NO_WG, void, env, i32)
Why the change to NO_WG ? The code change doesn't remove any
writes to globals...
> DEF_HELPER_1(yield, void, env)
> DEF_HELPER_1(pre_hvc, void, env)
> DEF_HELPER_2(pre_smc, void, env, i32)
> diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
> index aa14f15eb62..28a1c7243ad 100644
> --- a/target/arm/tcg/op_helper.c
> +++ b/target/arm/tcg/op_helper.c
> @@ -409,7 +409,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
> #endif
> }
>
> -void HELPER(wfit)(CPUARMState *env, uint64_t timeout)
> +void HELPER(wfit)(CPUARMState *env, uint32_t rd)
> {
> #ifdef CONFIG_USER_ONLY
> /*
> @@ -428,6 +428,8 @@ void HELPER(wfit)(CPUARMState *env, uint64_t timeout)
> int target_el = check_wfx_trap(env, false, &excp);
> /* The WFIT should time out when CNTVCT_EL0 >= the specified value. */
> uint64_t cntval = gt_get_countervalue(env);
> + uint64_t timeout = env->xregs[rd];
> +
> /*
> * We want the value that we would get if we read CNTVCT_EL0 from
> * the current exception level, so the direct_access offset, not
> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
> index 5d261a5e32b..073454b9195 100644
> --- a/target/arm/tcg/translate-a64.c
> +++ b/target/arm/tcg/translate-a64.c
> @@ -2064,7 +2064,7 @@ static bool trans_WFIT(DisasContext *s, arg_WFIT *a)
> }
>
> gen_a64_update_pc(s, 4);
> - gen_helper_wfit(tcg_env, cpu_reg(s, a->rd));
> + gen_helper_wfit(tcg_env, tcg_constant_i32(a->rd));
> /* Go back to the main loop to check for interrupts */
> s->base.is_jmp = DISAS_EXIT;
> return true;
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
You might squash this in to patch 4 if you like.
thanks
-- PMM
Peter Maydell <peter.maydell@linaro.org> writes:
> On Tue, 24 Feb 2026 at 15:43, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> If we generate an exception for WFIT we could also fill out the rv/rn
>> fields of the ISS. To facilitate this pass the register number
>> directly and read the timeout value in the helper itself.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> target/arm/tcg/helper-defs.h | 2 +-
>> target/arm/tcg/op_helper.c | 4 +++-
>> target/arm/tcg/translate-a64.c | 2 +-
>> 3 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/arm/tcg/helper-defs.h b/target/arm/tcg/helper-defs.h
>> index 5a10a9fba3b..a9a062cf777 100644
>> --- a/target/arm/tcg/helper-defs.h
>> +++ b/target/arm/tcg/helper-defs.h
>> @@ -55,7 +55,7 @@ DEF_HELPER_2(exception_pc_alignment, noreturn, env, vaddr)
>> DEF_HELPER_1(setend, void, env)
>> DEF_HELPER_2(wfi, void, env, i32)
>> DEF_HELPER_1(wfe, void, env)
>> -DEF_HELPER_2(wfit, void, env, i64)
>> +DEF_HELPER_FLAGS_2(wfit, TCG_CALL_NO_WG, void, env, i32)
>
> Why the change to NO_WG ? The code change doesn't remove any
> writes to globals...
I thought that was the right way round - it doesn't write to any
globals, but it does read so the helper needs to ensure globals are
synced before the callback.
>
>> DEF_HELPER_1(yield, void, env)
>> DEF_HELPER_1(pre_hvc, void, env)
>> DEF_HELPER_2(pre_smc, void, env, i32)
>> diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
>> index aa14f15eb62..28a1c7243ad 100644
>> --- a/target/arm/tcg/op_helper.c
>> +++ b/target/arm/tcg/op_helper.c
>> @@ -409,7 +409,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
>> #endif
>> }
>>
>> -void HELPER(wfit)(CPUARMState *env, uint64_t timeout)
>> +void HELPER(wfit)(CPUARMState *env, uint32_t rd)
>> {
>> #ifdef CONFIG_USER_ONLY
>> /*
>> @@ -428,6 +428,8 @@ void HELPER(wfit)(CPUARMState *env, uint64_t timeout)
>> int target_el = check_wfx_trap(env, false, &excp);
>> /* The WFIT should time out when CNTVCT_EL0 >= the specified value. */
>> uint64_t cntval = gt_get_countervalue(env);
>> + uint64_t timeout = env->xregs[rd];
>> +
>> /*
>> * We want the value that we would get if we read CNTVCT_EL0 from
>> * the current exception level, so the direct_access offset, not
>> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
>> index 5d261a5e32b..073454b9195 100644
>> --- a/target/arm/tcg/translate-a64.c
>> +++ b/target/arm/tcg/translate-a64.c
>> @@ -2064,7 +2064,7 @@ static bool trans_WFIT(DisasContext *s, arg_WFIT *a)
>> }
>>
>> gen_a64_update_pc(s, 4);
>> - gen_helper_wfit(tcg_env, cpu_reg(s, a->rd));
>> + gen_helper_wfit(tcg_env, tcg_constant_i32(a->rd));
>> /* Go back to the main loop to check for interrupts */
>> s->base.is_jmp = DISAS_EXIT;
>> return true;
>
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> You might squash this in to patch 4 if you like.
>
> thanks
> -- PMM
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
On Thu, 26 Feb 2026 at 10:33, Alex Bennée <alex.bennee@linaro.org> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Tue, 24 Feb 2026 at 15:43, Alex Bennée <alex.bennee@linaro.org> wrote: > >> > >> If we generate an exception for WFIT we could also fill out the rv/rn > >> fields of the ISS. To facilitate this pass the register number > >> directly and read the timeout value in the helper itself. > >> > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >> --- > >> target/arm/tcg/helper-defs.h | 2 +- > >> target/arm/tcg/op_helper.c | 4 +++- > >> target/arm/tcg/translate-a64.c | 2 +- > >> 3 files changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/target/arm/tcg/helper-defs.h b/target/arm/tcg/helper-defs.h > >> index 5a10a9fba3b..a9a062cf777 100644 > >> --- a/target/arm/tcg/helper-defs.h > >> +++ b/target/arm/tcg/helper-defs.h > >> @@ -55,7 +55,7 @@ DEF_HELPER_2(exception_pc_alignment, noreturn, env, vaddr) > >> DEF_HELPER_1(setend, void, env) > >> DEF_HELPER_2(wfi, void, env, i32) > >> DEF_HELPER_1(wfe, void, env) > >> -DEF_HELPER_2(wfit, void, env, i64) > >> +DEF_HELPER_FLAGS_2(wfit, TCG_CALL_NO_WG, void, env, i32) > > > > Why the change to NO_WG ? The code change doesn't remove any > > writes to globals... > > I thought that was the right way round - it doesn't write to any > globals, but it does read so the helper needs to ensure globals are > synced before the callback. The default if you use a DEF_HELPER macro that doesn't specify flags is "reading and writing to globals is OK" (it's a zero flags argument). So this change goes from "flags 0" to "flags NO_WG" and doesn't make any difference to how TCG thinks of its behaviour w.r.t. reading globals. I think you're right that the helper doesn't need to be able to write globals, but making a stricter promise about what the helper does is something that's a distinct change from adding the rn info to the syndrome. -- PMM
© 2016 - 2026 Red Hat, Inc.