Re: [PATCH] hw/timer/a9gtimer: Clear pending interrupt, after the clear of Event flag
On 6/26/20 4:46 PM, Peter Maydell wrote: On Tue, 16 Jun 2020 at 08:04, Václav Vanc wrote: On 6/15/20 1:04 PM, Peter Maydell wrote: 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. That's interesting. Which version of the Cortex-A9 does this board have? The TRM documents that the comparator behaviour changed in r2p0... thanks -- PMM It is "SABRE Lite (i.MX6)" board with Cortex-A9 r2p10. Behavior is also the same for "Xilinx Zynq-7000 SoC ZC702 Evaluation Kit" with Cortex-A9 r3p0. Unfortunately I do not have any board with older revision of Cortex-A9. 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.v...@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
Re: [PATCH] hw/timer/a9gtimer: Clear pending interrupt, after the clear of Event flag
On Tue, 16 Jun 2020 at 08:04, Václav Vanc wrote: > > On 6/15/20 1:04 PM, Peter Maydell wrote: > 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. That's interesting. Which version of the Cortex-A9 does this board have? The TRM documents that the comparator behaviour changed in r2p0... thanks -- PMM
Re: [PATCH] hw/timer/a9gtimer: Clear pending interrupt, after the clear of Event flag
On 6/15/20 1:04 PM, Peter Maydell wrote: On Wed, 10 Jun 2020 at 09:47, Vaclav Vanc 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 --- 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.v...@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
Re: [PATCH] hw/timer/a9gtimer: Clear pending interrupt, after the clear of Event flag
On Wed, 10 Jun 2020 at 09:47, Vaclav Vanc 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 > --- > 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
[PATCH] hw/timer/a9gtimer: Clear pending interrupt, after the clear of Event flag
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 --- 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; case R_COMPARATOR_HI: shift = 32; -- 2.20.1