Re: [PATCH 3/5] target/arm: trigger timer recalc on SCR:ECVEN change

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 6/9/26 07:00, Alex Bennée wrote:
>> Toggling the ECVEN state affects the offset calculated in
>> gt_phys_raw_cnt_offset so we should trigger a re-calculation on its
>> change.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   target/arm/helper.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 5c640853060..22f5117ee54 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -37,6 +37,9 @@
>>   #include "qemu/plugin.h"
>>     static void switch_mode(CPUARMState *env, int mode);
>> +#ifndef CONFIG_USER_ONLY
>> +static void gt_recalc_timer(ARMCPU *cpu, int timeridx);
>> +#endif
>>     int compare_u64(const void *a, const void *b)
>>   {
>> @@ -821,6 +824,12 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>>       changed = env->cp15.scr_el3 ^ value;
>>       env->cp15.scr_el3 = value;
>>   +#ifndef CONFIG_USER_ONLY
>> +    if (changed & SCR_ECVEN) {
>> +        gt_recalc_timer(cpu, GTIMER_PHYS);
>> +    }
>> +#endif
>> +
>>       /*
>>        * If SCR_EL3.{NS,NSE} changes, i.e. change of security state,
>>        * we must invalidate all TLBs below EL3.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> As an aside, gt_phys_raw_cnt_offset is missing a check on EL3
> existing, before the check for ECVEn set.

Not many of the references to cp15.scr_el3 seem to have those checks
(although I guess that may be inferred from higher up the call chain).

I can add a arm_feature(env, ARM_FEATURE_EL3) in front or maybe we want
something similar to arm_is_el2_enabled?

>
>
> r~

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro