From: Axel Heider <axel.heider@hensoldt.net>

Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
---
See https://gitlab.com/qemu-project/qemu/-/issues/1263
When running the seL4 tests
(https://docs.sel4.systems/projects/sel4test), on the sabrelight
platform the timer test fails (and thus it's disabled by default).
Investigation has shown that the arm/imx6 EPIT timer interrupt does not
fire properly, instead of a second in can take up to a minute to finally
see the interrupt.

 hw/timer/imx_epit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 2bf8c754b2..0b13c1eab0 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -276,9 +276,12 @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
             ptimer_set_count(s->timer_reload, s->lr);
         }
 
+        // commit s->timer_reload before imx_epit_reload_compare_timer
+        // as timer_reload is read in imx_epit_reload_compare_timer
+        ptimer_transaction_commit(s->timer_reload);
+
         imx_epit_reload_compare_timer(s);
         ptimer_transaction_commit(s->timer_cmp);
-        ptimer_transaction_commit(s->timer_reload);
         break;
 
     case 3: /* CMP */
-- 
2.34.5

On Fri, 21 Oct 2022 at 20:18, ~axelheider <axelheider@git.sr.ht> wrote:
>
> From: Axel Heider <axel.heider@hensoldt.net>
>
> Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
> ---
> See https://gitlab.com/qemu-project/qemu/-/issues/1263
> When running the seL4 tests
> (https://docs.sel4.systems/projects/sel4test), on the sabrelight
> platform the timer test fails (and thus it's disabled by default).
> Investigation has shown that the arm/imx6 EPIT timer interrupt does not
> fire properly, instead of a second in can take up to a minute to finally
> see the interrupt.
>
>  hw/timer/imx_epit.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
> index 2bf8c754b2..0b13c1eab0 100644
> --- a/hw/timer/imx_epit.c
> +++ b/hw/timer/imx_epit.c
> @@ -276,9 +276,12 @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
>              ptimer_set_count(s->timer_reload, s->lr);
>          }
>
> +        // commit s->timer_reload before imx_epit_reload_compare_timer
> +        // as timer_reload is read in imx_epit_reload_compare_timer

QEMU coding style requires /* ... */ for comments, not //
(with the /* and */ on lines of their own if it's a multiline
comment.

> +        ptimer_transaction_commit(s->timer_reload);
> +
>          imx_epit_reload_compare_timer(s);
>          ptimer_transaction_commit(s->timer_cmp);
> -        ptimer_transaction_commit(s->timer_reload);
>          break;

Yes, I see what's happening here. It's OK to commit the timer_reload
timer transaction first because that timer doesn't care about the
timer_cmp timer.

Do we also need to change the other places that call
imx_epit_reload_compare_timer() in the handling of CR
register writes ? (Those are a little more tricky.)

thanks
-- PMM

On Mon, 24 Oct 2022 at 13:37, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 21 Oct 2022 at 20:18, ~axelheider <axelheider@git.sr.ht> wrote:
> >
> > From: Axel Heider <axel.heider@hensoldt.net>
> >
> > Signed-off-by: Axel Heider <axel.heider@hensoldt.net>
> > ---
> > See https://gitlab.com/qemu-project/qemu/-/issues/1263
> > When running the seL4 tests
> > (https://docs.sel4.systems/projects/sel4test), on the sabrelight
> > platform the timer test fails (and thus it's disabled by default).
> > Investigation has shown that the arm/imx6 EPIT timer interrupt does not
> > fire properly, instead of a second in can take up to a minute to finally
> > see the interrupt.

Oh yes, this sort of information should ideally be in the commit
message proper -- the commit message is the place to explain what
you're changing and why. You can make the gitlab issue auto-close
by putting a line

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1263

into the commit message.

thanks
-- PMM