KVM: arm64: selftests: Deal with spurious timer interrupts

Make sure the timer test can properly handle a spurious timer
interrupt, something that is far from being unlikely.

This involves checking for the GIC IAR return value (don't bother
handling the interrupt if it was spurious) as well as the timer
control register (don't do anything if the interrupt is masked
or the timer disabled). Take this opportunity to rewrite the
timer handler in a more readable way.

This solves a bunch of failures that creep up on systems that
are slow to retire the interrupt, something that the GIC architecture
makes no guarantee about.

Reviewed-by: Colton Lewis <coltonlewis@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20230330174800.2677007-20-maz@kernel.org

diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
index 26556a2..176ab41 100644 (file)
--- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -121,25 +121,35 @@ static void guest_validate_irq(unsigned int intid,
        uint64_t xcnt = 0, xcnt_diff_us, cval = 0;
        unsigned long xctl = 0;
        unsigned int timer_irq = 0;
+       unsigned int accessor;
 
-       if (stage == GUEST_STAGE_VTIMER_CVAL ||
-               stage == GUEST_STAGE_VTIMER_TVAL) {
-               xctl = timer_get_ctl(VIRTUAL);
-               timer_set_ctl(VIRTUAL, CTL_IMASK);
-               xcnt = timer_get_cntct(VIRTUAL);
-               cval = timer_get_cval(VIRTUAL);
+       if (intid == IAR_SPURIOUS)
+               return;
+
+       switch (stage) {
+       case GUEST_STAGE_VTIMER_CVAL:
+       case GUEST_STAGE_VTIMER_TVAL:
+               accessor = VIRTUAL;
                timer_irq = vtimer_irq;
-       } else if (stage == GUEST_STAGE_PTIMER_CVAL ||
-               stage == GUEST_STAGE_PTIMER_TVAL) {
-               xctl = timer_get_ctl(PHYSICAL);
-               timer_set_ctl(PHYSICAL, CTL_IMASK);
-               xcnt = timer_get_cntct(PHYSICAL);
-               cval = timer_get_cval(PHYSICAL);
+               break;
+       case GUEST_STAGE_PTIMER_CVAL:
+       case GUEST_STAGE_PTIMER_TVAL:
+               accessor = PHYSICAL;
                timer_irq = ptimer_irq;
-       } else {
+               break;
+       default:
                GUEST_ASSERT(0);
+               return;
        }
 
+       xctl = timer_get_ctl(accessor);
+       if ((xctl & CTL_IMASK) || !(xctl & CTL_ENABLE))
+               return;
+
+       timer_set_ctl(accessor, CTL_IMASK);
+       xcnt = timer_get_cntct(accessor);
+       cval = timer_get_cval(accessor);
+
        xcnt_diff_us = cycles_to_usec(xcnt - shared_data->xcnt);
 
        /* Make sure we are dealing with the correct timer IRQ */
@@ -148,6 +158,8 @@ static void guest_validate_irq(unsigned int intid,
        /* Basic 'timer condition met' check */
        GUEST_ASSERT_3(xcnt >= cval, xcnt, cval, xcnt_diff_us);
        GUEST_ASSERT_1(xctl & CTL_ISTATUS, xctl);
+
+       WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1);
 }
 
 static void guest_irq_handler(struct ex_regs *regs)
@@ -158,8 +170,6 @@ static void guest_irq_handler(struct ex_regs *regs)
 
        guest_validate_irq(intid, shared_data);
 
-       WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1);
-
        gic_set_eoi(intid);
 }