Re: [kvm-unit-tests PATCH] arm64: timer: ignore ISTATUS with disabled timer

[Andrew Jones <andrew.jones@linux.dev>]

On Wed, Jun 14, 2023 at 05:38:32PM -0700, Nadav Amit wrote:
> From: Nadav Amit <nadav.amit@gmail.com>
> 
> According to ARM specifications for the vtimer (CNTV_CTL_EL0): "When the
> value of the ENABLE bit is 0, the ISTATUS field is UNKNOWN."
> 
> Currently the test, however, does check that ISTATUS is cleared when the
> ENABLE bit is zero. Remove this check as the value is unknown.

Hmm, does it? timer_pending() is

 /* Check that the timer condition is met. */
 static bool timer_pending(struct timer_info *info)
 {
        return (info->read_ctl() & ARCH_TIMER_CTL_ENABLE) &&
                (info->read_ctl() & ARCH_TIMER_CTL_ISTATUS);
 }

which I would expect to short-circuit when ENABLE is zero.

Thanks,
drew

> 
> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---
>  arm/timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arm/timer.c b/arm/timer.c
> index 2cb8051..0b976a7 100644
> --- a/arm/timer.c
> +++ b/arm/timer.c
> @@ -256,7 +256,7 @@ static void test_timer_pending(struct timer_info *info)
>  	set_timer_irq_enabled(info, true);
>  
>  	report(!info->irq_received, "no interrupt when timer is disabled");
> -	report(!timer_pending(info) && gic_timer_check_state(info, GIC_IRQ_STATE_INACTIVE),
> +	report(gic_timer_check_state(info, GIC_IRQ_STATE_INACTIVE),
>  			"interrupt signal no longer pending");
>  
>  	info->write_cval(now - 1);
> -- 
> 2.34.1
>