All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	boost.lists@gmail.com, victor.clement@openwide.fr,
	"open list:ARM TCG CPUs" <qemu-arm@nongnu.org>,
	pavel.dovgaluk@ispras.ru, pbonzini@redhat.com,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: [PATCH  v1 2/2] target/arm: only set the nexttick timer if !ISTATUS
Date: Tue, 28 Jul 2020 15:10:05 +0100	[thread overview]
Message-ID: <20200728141005.28664-3-alex.bennee@linaro.org> (raw)
In-Reply-To: <20200728141005.28664-1-alex.bennee@linaro.org>

Otherwise we have an unfortunate interaction with -count sleep=off
which means we fast forward time when we don't need to. The easiest
way to trigger it was to attach to the gdbstub and place a break point
at the timers IRQ routine. Once the timer fired setting the next event
at INT_MAX then qemu_start_warp_timer would skip to the end.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/helper.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index c69a2baf1d3..ec1b84cf0fd 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2683,7 +2683,7 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
         uint64_t count = gt_get_countervalue(&cpu->env);
         /* Note that this must be unsigned 64 bit arithmetic: */
         int istatus = count - offset >= gt->cval;
-        uint64_t nexttick;
+        uint64_t nexttick = 0;
         int irqstate;
 
         gt->ctl = deposit32(gt->ctl, 2, 1, istatus);
@@ -2692,21 +2692,30 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
         qemu_set_irq(cpu->gt_timer_outputs[timeridx], irqstate);
 
         if (istatus) {
-            /* Next transition is when count rolls back over to zero */
-            nexttick = UINT64_MAX;
+            /*
+             * The IRQ status of the timer will persist until:
+             *   - CVAL is changed or
+             *   - ENABLE is changed
+             *
+             * There is no point re-arming the timer for some far
+             * flung future - currently it just is.
+             */
+            timer_del(cpu->gt_timer[timeridx]);
         } else {
             /* Next transition is when we hit cval */
             nexttick = gt->cval + offset;
-        }
-        /* Note that the desired next expiry time might be beyond the
-         * signed-64-bit range of a QEMUTimer -- in this case we just
-         * set the timer for as far in the future as possible. When the
-         * timer expires we will reset the timer for any remaining period.
-         */
-        if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
-            timer_mod_ns(cpu->gt_timer[timeridx], INT64_MAX);
-        } else {
-            timer_mod(cpu->gt_timer[timeridx], nexttick);
+
+            /*
+             * It is possible the next tick is beyond the
+             * signed-64-bit range of a QEMUTimer but currently the
+             * timer system doesn't support a run time of more the 292
+             * odd years so we set it to INT_MAX in this case.
+             */
+            if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
+                timer_mod_ns(cpu->gt_timer[timeridx], INT64_MAX);
+            } else {
+                timer_mod(cpu->gt_timer[timeridx], nexttick);
+            }
         }
         trace_arm_gt_recalc(timeridx, irqstate, nexttick);
     } else {
-- 
2.20.1



  parent reply	other threads:[~2020-07-28 14:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28 14:10 [PATCH v1 0/2] clean-ups for sleep=off behaviour Alex Bennée
2020-07-28 14:10 ` [PATCH v1 1/2] qemu-timer: gracefully handle the end of time Alex Bennée
2020-07-28 14:42   ` Paolo Bonzini
2020-07-28 16:08     ` Alex Bennée
2020-07-28 16:58       ` Paolo Bonzini
2020-07-28 14:10 ` Alex Bennée [this message]
2020-07-28 14:16   ` [PATCH v1 2/2] target/arm: only set the nexttick timer if !ISTATUS Peter Maydell
2020-07-28 16:11     ` Alex Bennée
2020-07-28 16:23       ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200728141005.28664-3-alex.bennee@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=boost.lists@gmail.com \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=victor.clement@openwide.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.