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

[Václav Vanc]

On 6/15/20 1:04 PM, Peter Maydell wrote:
> 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.
Hi, thanks for reviewing the patch.

> 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.

We must distinguish between two cases:
1, Auto-increment is disabled.
I just run some test on SABRE Lite (i.MX6) board.
I had auto-increment disabled, I verified, that GIC is configured for 
Edge interrupts. Once count went past the compare value I got the 
interrupt. I did not touch Timer registers, just signal EOI to GIC and 
surprisingly, I got a another interrupt. If I stopped the timer 
interrupts stopped coming (Status was still set to 1).

From this behavior I assume, that every time the Timer is incremented 
(and Timer value is past the compare value) an EDGE interrupt (that 
means actual X->0->1 transition) is asserted. This is really interesting 
from HW point of view. This would mean, that a9_gtimer_update function 
should generate the pulse and not level on compare event.

2, Auto-increment is enabled.
This is actually what the patch was aiming for.
First call of a9_gtimer_update will potentially update the compare 
value, then the interrupt and event flag is de-asserted. Second call of 
a9_gtimer_update will just update the timer (or re-assert the interrupt 
if the Timer hit the compare value in the meantime, which is fine).

Let me know I you want me to run some more tests.

> (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

Best Regards,
Ing. Václav Vanc
Project Engineer

SYSGO s.r.o.
Embedding Innovations
Research and Development Center Prague
Zeleny pruh 1560/99 I CZ-14000 Praha 4
Phone: +420 222138 828, +49 6136 9948 828
Fax: +420 244911174
E-mail: vaclav.vanc@sysgo.com
_________________________________________________________________________________

Web: https://www.sysgo.com
Blog: https://www.sysgo.com/blog
Events: https://www.sysgo.com/events
Newsletter: https://www.sysgo.com/newsletter
_________________________________________________________________________________

Handelsregister/Commercial Registry: HRB Mainz 90 HRB 48884
Geschäftsführung/Managing Directors: Etienne Butery (CEO), Kai Sablotny 
(COO)
USt-Id-Nr./VAT-Id-No.: DE 149062328

The protection of your personal data is important to us. Under the 
following link you can see the information in accordance with article 13 
GDPR: https://www.sysgo.com/privacy_policy