Re: [PATCH] hw/timer/a9gtimer: Clear pending interrupt, after the clear of Event flag

[Peter Maydell]

On Wed, 10 Jun 2020 at 09:47, Vaclav Vanc <vav@sysgo.com> wrote:
>
> A9 Global Timer is used with Edge triggered interrupts (This is true
> at least for Zynq and i.MX6 processors).
> When Event Flag is cleared in Interrupt Status Register and new interrupt
> was supposed to be scheduled, interrupt request is never cleared.
> Since interrupt in GIC is configured as Edge triggered, new interrupts
> are not registered (because interrupt is stuck at pending and GIC thinks
> it was already serviced). As a result interrupts from timer does not work
> anymore.
>
> Note: This happens only when interrupt was not serviced before the next
> interrupt is suppose to be scheduled. This happens for example during
> the increased load of the host system.
>
> Interrupt is now always cleared when Event Flag is cleared.
> This is in accordance to A9 Global Timer documentation.
>
> Signed-off-by: Vaclav Vanc <vav@sysgo.com>
> ---
>  hw/timer/a9gtimer.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
> index 7233068a37..732889105e 100644
> --- a/hw/timer/a9gtimer.c
> +++ b/hw/timer/a9gtimer.c
> @@ -206,6 +206,9 @@ static void a9_gtimer_write(void *opaque, hwaddr addr, 
> uint64_t value,
>      case R_INTERRUPT_STATUS:
>          a9_gtimer_update(s, false);
>          gtb->status &= ~value;
> +        if (gtb->status == 0) {
> +            qemu_set_irq(gtb->irq, 0);
> +        }
>          break;

Hi; thanks for this patch. I can see the situation you're trying to address,
but I can't find anything in the docs that convinces me that this change
is the right way to fix it.

The problem we have is that the a9_gtimer_update() function (which is
going to get called after this code at the end of the a9_gtimer_write()
function) updates the gtb->status bit based on whether the timer has
currently passed the compare value. So effectively we do the "has the
count gone past the compare value" check not only when the timer expires
but also at every register write. My best guess is that the real hardware
only does an expiry-check when it does a counter value update. If that's
the case then in the situation you outline the guest clearing the event
flag should mean that the interrupt is not re-asserted until the counter
next increments past the compare value (ie not for a little while) whereas
with your patch it will be instantly re-asserted.

(Unfortunately the A9 TRM is not clear on the behaviour here. It would
probably be possible to write some test code to check the real h/w
behaviour.)

thanks
-- PMM