Re: [PATCH 5/5] target/arm/helper: Implement CNTHCTL_EL2.CNT[VP]MASK

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 19 Jul 2023 at 16:56, Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> When FEAT_RME is implemented, these bits override the value of
> CNT[VP]_CTL_EL0.IMASK in Realm and Root state.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  target/arm/helper.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 2017b11795..5b173a827f 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -2608,6 +2608,23 @@ static uint64_t gt_get_countervalue(CPUARMState *env)
>      return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / gt_cntfrq_period_ns(cpu);
>  }
>
> +static bool gt_is_masked(CPUARMState *env, int timeridx)
> +{
> +    ARMSecuritySpace ss = arm_security_space(env);
> +
> +    /*
> +     * If bits CNTHCTL_EL2.CNT[VP]MASK are set, they override
> +     * CNT[VP]_CTL_EL0.IMASK. They are RES0 in Secure and NonSecure state.
> +     */
> +    if ((ss == ARMSS_Root || ss == ARMSS_Realm) &&
> +        ((timeridx == GTIMER_VIRT && extract64(env->cp15.cnthctl_el2, 18, 1)) ||
> +         (timeridx == GTIMER_PHYS && extract64(env->cp15.cnthctl_el2, 19, 1)))) {
> +        return true;
> +    }
> +
> +    return env->cp15.c14_timer[timeridx].ctl & 2;
> +}
> +
>  static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
>  {
>      ARMGenericTimer *gt = &cpu->env.cp15.c14_timer[timeridx];
> @@ -2627,7 +2644,7 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
>
>          gt->ctl = deposit32(gt->ctl, 2, 1, istatus);
>
> -        irqstate = (istatus && !(gt->ctl & 2));
> +        irqstate = (istatus && !gt_is_masked(&cpu->env, timeridx));
>          qemu_set_irq(cpu->gt_timer_outputs[timeridx], irqstate);
>
>          if (istatus) {
> @@ -2759,7 +2776,7 @@ static void gt_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri,
>           * IMASK toggled: don't need to recalculate,
>           * just set the interrupt line based on ISTATUS
>           */
> -        int irqstate = (oldval & 4) && !(value & 2);
> +        int irqstate = (oldval & 4) && !gt_is_masked(env, timeridx);
>
>          trace_arm_gt_imask_toggle(timeridx, irqstate);
>          qemu_set_irq(cpu->gt_timer_outputs[timeridx], irqstate);

If these CNTHCTL bits now affect whether the timer interrupts
are masked, then we need to update the timer irq state
on writes to CNTHCTL that change the bits.

thanks
-- PMM