All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion
@ 2016-09-07 13:22 Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 01/16] hw/ptimer: Actually stop the timer in case of error Dmitry Osipenko
                   ` (16 more replies)
  0 siblings, 17 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Hello,

Currently, QEMU ARM MPTimer device model provides only a certain subset of
the emulation behavior. This patch series is supposed to add missing parts by
converting the MPTimer to use generic ptimer helper. It fixes some important
ptimer bugs and provides new features that are required for the ARM MPTimer.

Changelog:
    V16: Suppressed ptimer warning messages under qtest.

         Added a comment for the default policy in the "include/hw/ptimer.h"

         Patches "Change ptimer_get_count to return "1" for the expired timer"
         and "Fix counter - 1 returned by ptimer_get_count for the active timer"
         are merged and converted into a "no counter round down" ptimer policy.

         Added "hw/timer/arm_mptimer.c" into the "make check" GCOV.

    V15: As per Peter's Maydell request: split ptimer policy patches,
         so that first "policy" patch only sets default policy to all
         of the timers without introducing new behaviour policies.

         The "deferred trigger" ptimer policy from V14 is converted to
         "continuous trigger" as it is more intuitive.

         New ptimer policies added.

         New ptimer patch "Actually stop timer in case of error".

         Fixed ARM MPTimer periodic/oneshot "on the fly" mode switch
         for the case when load = 0 and new mode has prescaler = 0.

         Added QTests for each of the ptimer policies.

         Added QTests for the ARM MPTimer.

    V14: Set the ptimer policy in the ptimer_init() instead of adding
         ptimer_set_policy(), keeping ptimer VMState unchanged and dropped
         hw_error() hardening asserts as per Peter's Maydell V13 review
         comments, addressed the rest of the review comments.

    V13-V2: https://lists.gnu.org/archive/html/qemu-arm/2016-05/msg00269.html

Dmitry Osipenko (16):
  hw/ptimer: Actually stop the timer in case of error
  hw/ptimer: Introduce timer policy feature
  tests: Add ptimer tests
  hw/ptimer: Suppress error messages under qtest
  hw/ptimer: Add "wraparound after one period" policy
  tests: ptimer: Add tests for "wraparound after one period" policy
  hw/ptimer: Add "continuous trigger" policy
  tests: ptimer: Add tests for "continuous trigger" policy
  hw/ptimer: Add "no immediate trigger" policy
  tests: ptimer: Add tests for "no immediate trigger" policy
  hw/ptimer: Add "no immediate reload" policy
  tests: ptimer: Add tests for "no immediate reload" policy
  hw/ptimer: Add "no counter round down" policy
  tests: ptimer: Add tests for "no counter round down" policy
  arm_mptimer: Convert to use ptimer
  tests: Add tests for the ARM MPTimer

 hw/arm/musicpal.c              |    2 +-
 hw/core/ptimer.c               |   93 +++-
 hw/dma/xilinx_axidma.c         |    2 +-
 hw/m68k/mcf5206.c              |    2 +-
 hw/m68k/mcf5208.c              |    2 +-
 hw/net/fsl_etsec/etsec.c       |    2 +-
 hw/net/lan9118.c               |    2 +-
 hw/timer/allwinner-a10-pit.c   |    2 +-
 hw/timer/arm_mptimer.c         |  149 +++---
 hw/timer/arm_timer.c           |    2 +-
 hw/timer/digic-timer.c         |    2 +-
 hw/timer/etraxfs_timer.c       |    6 +-
 hw/timer/exynos4210_mct.c      |    7 +-
 hw/timer/exynos4210_pwm.c      |    2 +-
 hw/timer/exynos4210_rtc.c      |    4 +-
 hw/timer/grlib_gptimer.c       |    2 +-
 hw/timer/imx_epit.c            |    4 +-
 hw/timer/imx_gpt.c             |    2 +-
 hw/timer/lm32_timer.c          |    2 +-
 hw/timer/milkymist-sysctl.c    |    4 +-
 hw/timer/puv3_ost.c            |    2 +-
 hw/timer/sh_timer.c            |    2 +-
 hw/timer/slavio_timer.c        |    2 +-
 hw/timer/xilinx_timer.c        |    2 +-
 include/hw/ptimer.h            |   43 +-
 include/hw/timer/arm_mptimer.h |    5 +-
 stubs/vmstate.c                |    5 +
 tests/Makefile.include         |    5 +
 tests/ptimer-test-stubs.c      |  107 ++++
 tests/ptimer-test.c            |  717 ++++++++++++++++++++++++++
 tests/ptimer-test.h            |   22 +
 tests/test-arm-mptimer.c       | 1105 ++++++++++++++++++++++++++++++++++++++++
 32 files changed, 2188 insertions(+), 122 deletions(-)
 create mode 100644 tests/ptimer-test-stubs.c
 create mode 100644 tests/ptimer-test.c
 create mode 100644 tests/ptimer-test.h
 create mode 100644 tests/test-arm-mptimer.c

-- 
2.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 01/16] hw/ptimer: Actually stop the timer in case of error
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 02/16] hw/ptimer: Introduce timer policy feature Dmitry Osipenko
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Running with counter / period = 0 is treated as a error case, printing error
message claiming that timer has been disabled. However, timer is only marked
as disabled, keeping to tick till expired and triggering after being claimed
as disabled. Stop the QEMU timer to avoid confusion.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 30829ee..02c3135 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -44,6 +44,7 @@ static void ptimer_reload(ptimer_state *s)
     }
     if (s->delta == 0 || s->period == 0) {
         fprintf(stderr, "Timer with period zero, disabling\n");
+        timer_del(s->timer);
         s->enabled = 0;
         return;
     }
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 02/16] hw/ptimer: Introduce timer policy feature
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 01/16] hw/ptimer: Actually stop the timer in case of error Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests Dmitry Osipenko
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Some of the timer devices may behave differently from what ptimer
provides. Introduce ptimer policy feature that allows ptimer users to
change default and wrong timer behaviour, for example to continuously
trigger periodic timer when load value is equal to "0".

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/arm/musicpal.c            |  2 +-
 hw/core/ptimer.c             |  4 +++-
 hw/dma/xilinx_axidma.c       |  2 +-
 hw/m68k/mcf5206.c            |  2 +-
 hw/m68k/mcf5208.c            |  2 +-
 hw/net/fsl_etsec/etsec.c     |  2 +-
 hw/net/lan9118.c             |  2 +-
 hw/timer/allwinner-a10-pit.c |  2 +-
 hw/timer/arm_timer.c         |  2 +-
 hw/timer/digic-timer.c       |  2 +-
 hw/timer/etraxfs_timer.c     |  6 +++---
 hw/timer/exynos4210_mct.c    |  7 ++++---
 hw/timer/exynos4210_pwm.c    |  2 +-
 hw/timer/exynos4210_rtc.c    |  4 ++--
 hw/timer/grlib_gptimer.c     |  2 +-
 hw/timer/imx_epit.c          |  4 ++--
 hw/timer/imx_gpt.c           |  2 +-
 hw/timer/lm32_timer.c        |  2 +-
 hw/timer/milkymist-sysctl.c  |  4 ++--
 hw/timer/puv3_ost.c          |  2 +-
 hw/timer/sh_timer.c          |  2 +-
 hw/timer/slavio_timer.c      |  2 +-
 hw/timer/xilinx_timer.c      |  2 +-
 include/hw/ptimer.h          | 24 +++++++++++++++++++++++-
 24 files changed, 56 insertions(+), 31 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index cc50ace..7527037 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -837,7 +837,7 @@ static void mv88w8618_timer_init(SysBusDevice *dev, mv88w8618_timer_state *s,
     s->freq = freq;
 
     bh = qemu_bh_new(mv88w8618_timer_tick, s);
-    s->ptimer = ptimer_init(bh);
+    s->ptimer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
 }
 
 static uint64_t mv88w8618_pit_read(void *opaque, hwaddr offset,
diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 02c3135..00fdf15 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -21,6 +21,7 @@ struct ptimer_state
     int64_t period;
     int64_t last_event;
     int64_t next_event;
+    uint8_t policy_mask;
     QEMUBH *bh;
     QEMUTimer *timer;
 };
@@ -243,12 +244,13 @@ const VMStateDescription vmstate_ptimer = {
     }
 };
 
-ptimer_state *ptimer_init(QEMUBH *bh)
+ptimer_state *ptimer_init(QEMUBH *bh, uint8_t policy_mask)
 {
     ptimer_state *s;
 
     s = (ptimer_state *)g_malloc0(sizeof(ptimer_state));
     s->bh = bh;
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ptimer_tick, s);
+    s->policy_mask = policy_mask;
     return s;
 }
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index a4753e5..b135a5f 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -548,7 +548,7 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
 
         st->nr = i;
         st->bh = qemu_bh_new(timer_hit, st);
-        st->ptimer = ptimer_init(st->bh);
+        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
         ptimer_set_freq(st->ptimer, s->freqhz);
     }
     return;
diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
index e14896e..b81901f 100644
--- a/hw/m68k/mcf5206.c
+++ b/hw/m68k/mcf5206.c
@@ -139,7 +139,7 @@ static m5206_timer_state *m5206_timer_init(qemu_irq irq)
 
     s = (m5206_timer_state *)g_malloc0(sizeof(m5206_timer_state));
     bh = qemu_bh_new(m5206_timer_trigger, s);
-    s->timer = ptimer_init(bh);
+    s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
     s->irq = irq;
     m5206_timer_reset(s);
     return s;
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 2415557..9240ebf 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -183,7 +183,7 @@ static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic)
     for (i = 0; i < 2; i++) {
         s = (m5208_timer_state *)g_malloc0(sizeof(m5208_timer_state));
         bh = qemu_bh_new(m5208_timer_trigger, s);
-        s->timer = ptimer_init(bh);
+        s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
         memory_region_init_io(&s->iomem, NULL, &m5208_timer_ops, s,
                               "m5208-timer", 0x00004000);
         memory_region_add_subregion(address_space, 0xfc080000 + 0x4000 * i,
diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index b5c777f..951c5f0 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -387,7 +387,7 @@ static void etsec_realize(DeviceState *dev, Error **errp)
 
 
     etsec->bh     = qemu_bh_new(etsec_timer_hit, etsec);
-    etsec->ptimer = ptimer_init(etsec->bh);
+    etsec->ptimer = ptimer_init(etsec->bh, PTIMER_POLICY_DEFAULT);
     ptimer_set_freq(etsec->ptimer, 100);
 }
 
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index 4615d87..3db8937 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -1345,7 +1345,7 @@ static int lan9118_init1(SysBusDevice *sbd)
     s->txp = &s->tx_packet;
 
     bh = qemu_bh_new(lan9118_tick, s);
-    s->timer = ptimer_init(bh);
+    s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
     ptimer_set_freq(s->timer, 10000);
     ptimer_set_limit(s->timer, 0xffff, 1);
 
diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
index 3385e5d..22ceabe 100644
--- a/hw/timer/allwinner-a10-pit.c
+++ b/hw/timer/allwinner-a10-pit.c
@@ -267,7 +267,7 @@ static void a10_pit_init(Object *obj)
         tc->container = s;
         tc->index = i;
         bh[i] = qemu_bh_new(a10_pit_timer_cb, tc);
-        s->timer[i] = ptimer_init(bh[i]);
+        s->timer[i] = ptimer_init(bh[i], PTIMER_POLICY_DEFAULT);
     }
 }
 
diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c
index 111a16d..98fddd7 100644
--- a/hw/timer/arm_timer.c
+++ b/hw/timer/arm_timer.c
@@ -171,7 +171,7 @@ static arm_timer_state *arm_timer_init(uint32_t freq)
     s->control = TIMER_CTRL_IE;
 
     bh = qemu_bh_new(arm_timer_tick, s);
-    s->timer = ptimer_init(bh);
+    s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
     vmstate_register(NULL, -1, &vmstate_arm_timer, s);
     return s;
 }
diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
index 0f21faf..e1fcf73 100644
--- a/hw/timer/digic-timer.c
+++ b/hw/timer/digic-timer.c
@@ -127,7 +127,7 @@ static void digic_timer_init(Object *obj)
 {
     DigicTimerState *s = DIGIC_TIMER(obj);
 
-    s->ptimer = ptimer_init(NULL);
+    s->ptimer = ptimer_init(NULL, PTIMER_POLICY_DEFAULT);
 
     /*
      * FIXME: there is no documentation on Digic timer
diff --git a/hw/timer/etraxfs_timer.c b/hw/timer/etraxfs_timer.c
index 36d8f46..8e18236 100644
--- a/hw/timer/etraxfs_timer.c
+++ b/hw/timer/etraxfs_timer.c
@@ -322,9 +322,9 @@ static int etraxfs_timer_init(SysBusDevice *dev)
     t->bh_t0 = qemu_bh_new(timer0_hit, t);
     t->bh_t1 = qemu_bh_new(timer1_hit, t);
     t->bh_wd = qemu_bh_new(watchdog_hit, t);
-    t->ptimer_t0 = ptimer_init(t->bh_t0);
-    t->ptimer_t1 = ptimer_init(t->bh_t1);
-    t->ptimer_wd = ptimer_init(t->bh_wd);
+    t->ptimer_t0 = ptimer_init(t->bh_t0, PTIMER_POLICY_DEFAULT);
+    t->ptimer_t1 = ptimer_init(t->bh_t1, PTIMER_POLICY_DEFAULT);
+    t->ptimer_wd = ptimer_init(t->bh_wd, PTIMER_POLICY_DEFAULT);
 
     sysbus_init_irq(dev, &t->irq);
     sysbus_init_irq(dev, &t->nmi);
diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
index ae69345..0c18934 100644
--- a/hw/timer/exynos4210_mct.c
+++ b/hw/timer/exynos4210_mct.c
@@ -1431,15 +1431,16 @@ static void exynos4210_mct_init(Object *obj)
 
     /* Global timer */
     bh[0] = qemu_bh_new(exynos4210_gfrc_event, s);
-    s->g_timer.ptimer_frc = ptimer_init(bh[0]);
+    s->g_timer.ptimer_frc = ptimer_init(bh[0], PTIMER_POLICY_DEFAULT);
     memset(&s->g_timer.reg, 0, sizeof(struct gregs));
 
     /* Local timers */
     for (i = 0; i < 2; i++) {
         bh[0] = qemu_bh_new(exynos4210_ltick_event, &s->l_timer[i]);
         bh[1] = qemu_bh_new(exynos4210_lfrc_event, &s->l_timer[i]);
-        s->l_timer[i].tick_timer.ptimer_tick = ptimer_init(bh[0]);
-        s->l_timer[i].ptimer_frc = ptimer_init(bh[1]);
+        s->l_timer[i].tick_timer.ptimer_tick =
+                                   ptimer_init(bh[0], PTIMER_POLICY_DEFAULT);
+        s->l_timer[i].ptimer_frc = ptimer_init(bh[1], PTIMER_POLICY_DEFAULT);
         s->l_timer[i].id = i;
     }
 
diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c
index 0e9e2e9..f576507 100644
--- a/hw/timer/exynos4210_pwm.c
+++ b/hw/timer/exynos4210_pwm.c
@@ -390,7 +390,7 @@ static void exynos4210_pwm_init(Object *obj)
     for (i = 0; i < EXYNOS4210_PWM_TIMERS_NUM; i++) {
         bh = qemu_bh_new(exynos4210_pwm_tick, &s->timer[i]);
         sysbus_init_irq(dev, &s->timer[i].irq);
-        s->timer[i].ptimer = ptimer_init(bh);
+        s->timer[i].ptimer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
         s->timer[i].id = i;
         s->timer[i].parent = s;
     }
diff --git a/hw/timer/exynos4210_rtc.c b/hw/timer/exynos4210_rtc.c
index da4dd45..1a648c5 100644
--- a/hw/timer/exynos4210_rtc.c
+++ b/hw/timer/exynos4210_rtc.c
@@ -555,12 +555,12 @@ static void exynos4210_rtc_init(Object *obj)
     QEMUBH *bh;
 
     bh = qemu_bh_new(exynos4210_rtc_tick, s);
-    s->ptimer = ptimer_init(bh);
+    s->ptimer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
     ptimer_set_freq(s->ptimer, RTC_BASE_FREQ);
     exynos4210_rtc_update_freq(s, 0);
 
     bh = qemu_bh_new(exynos4210_rtc_1Hz_tick, s);
-    s->ptimer_1Hz = ptimer_init(bh);
+    s->ptimer_1Hz = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
     ptimer_set_freq(s->ptimer_1Hz, RTC_BASE_FREQ);
 
     sysbus_init_irq(dev, &s->alm_irq);
diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
index dd000f5..712d1ae 100644
--- a/hw/timer/grlib_gptimer.c
+++ b/hw/timer/grlib_gptimer.c
@@ -363,7 +363,7 @@ static int grlib_gptimer_init(SysBusDevice *dev)
 
         timer->unit   = unit;
         timer->bh     = qemu_bh_new(grlib_gptimer_hit, timer);
-        timer->ptimer = ptimer_init(timer->bh);
+        timer->ptimer = ptimer_init(timer->bh, PTIMER_POLICY_DEFAULT);
         timer->id     = i;
 
         /* One IRQ line for each timer */
diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index eddf348..f34d7f7 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -314,10 +314,10 @@ static void imx_epit_realize(DeviceState *dev, Error **errp)
                           0x00001000);
     sysbus_init_mmio(sbd, &s->iomem);
 
-    s->timer_reload = ptimer_init(NULL);
+    s->timer_reload = ptimer_init(NULL, PTIMER_POLICY_DEFAULT);
 
     bh = qemu_bh_new(imx_epit_cmp, s);
-    s->timer_cmp = ptimer_init(bh);
+    s->timer_cmp = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
 }
 
 static void imx_epit_class_init(ObjectClass *klass, void *data)
diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
index 82bc73c..f034d65 100644
--- a/hw/timer/imx_gpt.c
+++ b/hw/timer/imx_gpt.c
@@ -461,7 +461,7 @@ static void imx_gpt_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(sbd, &s->iomem);
 
     bh = qemu_bh_new(imx_gpt_timeout, s);
-    s->timer = ptimer_init(bh);
+    s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
 }
 
 static void imx_gpt_class_init(ObjectClass *klass, void *data)
diff --git a/hw/timer/lm32_timer.c b/hw/timer/lm32_timer.c
index e45a65b..2a07b59 100644
--- a/hw/timer/lm32_timer.c
+++ b/hw/timer/lm32_timer.c
@@ -184,7 +184,7 @@ static void lm32_timer_init(Object *obj)
     sysbus_init_irq(dev, &s->irq);
 
     s->bh = qemu_bh_new(timer_hit, s);
-    s->ptimer = ptimer_init(s->bh);
+    s->ptimer = ptimer_init(s->bh, PTIMER_POLICY_DEFAULT);
 
     memory_region_init_io(&s->iomem, obj, &timer_ops, s,
                           "timer", R_MAX * 4);
diff --git a/hw/timer/milkymist-sysctl.c b/hw/timer/milkymist-sysctl.c
index 2194832..4488590 100644
--- a/hw/timer/milkymist-sysctl.c
+++ b/hw/timer/milkymist-sysctl.c
@@ -281,8 +281,8 @@ static void milkymist_sysctl_init(Object *obj)
 
     s->bh0 = qemu_bh_new(timer0_hit, s);
     s->bh1 = qemu_bh_new(timer1_hit, s);
-    s->ptimer0 = ptimer_init(s->bh0);
-    s->ptimer1 = ptimer_init(s->bh1);
+    s->ptimer0 = ptimer_init(s->bh0, PTIMER_POLICY_DEFAULT);
+    s->ptimer1 = ptimer_init(s->bh1, PTIMER_POLICY_DEFAULT);
 
     memory_region_init_io(&s->regs_region, obj, &sysctl_mmio_ops, s,
             "milkymist-sysctl", R_MAX * 4);
diff --git a/hw/timer/puv3_ost.c b/hw/timer/puv3_ost.c
index 93650b7..0b3d717 100644
--- a/hw/timer/puv3_ost.c
+++ b/hw/timer/puv3_ost.c
@@ -125,7 +125,7 @@ static int puv3_ost_init(SysBusDevice *dev)
     sysbus_init_irq(dev, &s->irq);
 
     s->bh = qemu_bh_new(puv3_ost_tick, s);
-    s->ptimer = ptimer_init(s->bh);
+    s->ptimer = ptimer_init(s->bh, PTIMER_POLICY_DEFAULT);
     ptimer_set_freq(s->ptimer, 50 * 1000 * 1000);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &puv3_ost_ops, s, "puv3_ost",
diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
index 255b2fc..9afb2d0 100644
--- a/hw/timer/sh_timer.c
+++ b/hw/timer/sh_timer.c
@@ -203,7 +203,7 @@ static void *sh_timer_init(uint32_t freq, int feat, qemu_irq irq)
     s->irq = irq;
 
     bh = qemu_bh_new(sh_timer_tick, s);
-    s->timer = ptimer_init(bh);
+    s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
 
     sh_timer_write(s, OFFSET_TCOR >> 2, s->tcor);
     sh_timer_write(s, OFFSET_TCNT >> 2, s->tcnt);
diff --git a/hw/timer/slavio_timer.c b/hw/timer/slavio_timer.c
index fb3e08b..bfee1f3 100644
--- a/hw/timer/slavio_timer.c
+++ b/hw/timer/slavio_timer.c
@@ -389,7 +389,7 @@ static int slavio_timer_init1(SysBusDevice *dev)
         tc->timer_index = i;
 
         bh = qemu_bh_new(slavio_timer_irq, tc);
-        s->cputimer[i].timer = ptimer_init(bh);
+        s->cputimer[i].timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT);
         ptimer_set_period(s->cputimer[i].timer, TIMER_PERIOD);
 
         size = i == 0 ? SYS_TIMER_SIZE : CPU_TIMER_SIZE;
diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 2ea970d..59439c0 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -218,7 +218,7 @@ static void xilinx_timer_realize(DeviceState *dev, Error **errp)
         xt->parent = t;
         xt->nr = i;
         xt->bh = qemu_bh_new(timer_hit, xt);
-        xt->ptimer = ptimer_init(xt->bh);
+        xt->ptimer = ptimer_init(xt->bh, PTIMER_POLICY_DEFAULT);
         ptimer_set_freq(xt->ptimer, t->freq_hz);
     }
 
diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index e397db5..7b0fc13 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -12,11 +12,33 @@
 #include "qemu/timer.h"
 #include "migration/vmstate.h"
 
+/* The default ptimer policy retains backward compatibility with the legacy
+ * timers. Custom policies are adjusting the default one. Consider providing
+ * a correct policy for your timer.
+ *
+ * The rough edges of the default policy:
+ *  - Starting to run with a period = 0 emits error message and stops the
+ *    timer without a trigger.
+ *
+ *  - Setting period to 0 of the running timer emits error message and
+ *    stops the timer without a trigger.
+ *
+ *  - Starting to run with counter = 0 or setting it to "0" while timer
+ *    is running causes a trigger and reloads counter with a limit value.
+ *    If limit = 0, ptimer emits error message and stops the timer.
+ *
+ *  - Counter value of the running timer is one less than the actual value.
+ *
+ *  - Changing period/frequency of the running timer loses time elapsed
+ *    since the last period, effectively restarting the timer with a
+ *    counter = counter value at the moment of change (.i.e. one less).  */
+#define PTIMER_POLICY_DEFAULT               0
+
 /* ptimer.c */
 typedef struct ptimer_state ptimer_state;
 typedef void (*ptimer_cb)(void *opaque);
 
-ptimer_state *ptimer_init(QEMUBH *bh);
+ptimer_state *ptimer_init(QEMUBH *bh, uint8_t policy_mask);
 void ptimer_set_period(ptimer_state *s, int64_t period);
 void ptimer_set_freq(ptimer_state *s, uint32_t freq);
 uint64_t ptimer_get_limit(ptimer_state *s);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 01/16] hw/ptimer: Actually stop the timer in case of error Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 02/16] hw/ptimer: Introduce timer policy feature Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 21:32   ` Eric Blake
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 04/16] hw/ptimer: Suppress error messages under qtest Dmitry Osipenko
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Ptimer is a generic countdown timer helper that is used by many timer
device models as well as by the QEMU core. Add QTests for the ptimer.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 stubs/vmstate.c           |   5 +
 tests/Makefile.include    |   2 +
 tests/ptimer-test-stubs.c | 107 +++++++++
 tests/ptimer-test.c       | 568 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/ptimer-test.h       |  22 ++
 5 files changed, 704 insertions(+)
 create mode 100644 tests/ptimer-test-stubs.c
 create mode 100644 tests/ptimer-test.c
 create mode 100644 tests/ptimer-test.h

diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 6590627..94b831e 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -3,6 +3,11 @@
 #include "migration/vmstate.h"
 
 const VMStateDescription vmstate_dummy = {};
+const VMStateInfo vmstate_info_uint8;
+const VMStateInfo vmstate_info_uint32;
+const VMStateInfo vmstate_info_uint64;
+const VMStateInfo vmstate_info_int64;
+const VMStateInfo vmstate_info_timer;
 
 int vmstate_register_with_alias_id(DeviceState *dev,
                                    int instance_id,
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14be491..82b7ae5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -274,6 +274,7 @@ check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
 check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
 
 check-qtest-generic-y += tests/qom-test$(EXESUF)
+check-qtest-generic-y += tests/ptimer-test$(EXESUF)
 
 qapi-schema += alternate-any.json
 qapi-schema += alternate-array.json
@@ -631,6 +632,7 @@ tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
 tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
 tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
+tests/ptimer-test$(EXESUF): tests/ptimer-test.o tests/ptimer-test-stubs.o hw/core/ptimer.o
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
 	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"  LINK  $(TARGET_DIR)$@")
diff --git a/tests/ptimer-test-stubs.c b/tests/ptimer-test-stubs.c
new file mode 100644
index 0000000..92a22fb
--- /dev/null
+++ b/tests/ptimer-test-stubs.c
@@ -0,0 +1,107 @@
+/*
+ * Stubs for the ptimer-test
+ *
+ * Author: Dmitry Osipenko <digetx@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "sysemu/replay.h"
+
+#include "ptimer-test.h"
+
+struct QEMUBH {
+    QEMUBHFunc *cb;
+    void *opaque;
+};
+
+QEMUTimerListGroup main_loop_tlg;
+
+int64_t ptimer_test_time_ns;
+
+void timer_init_tl(QEMUTimer *ts,
+                   QEMUTimerList *timer_list, int scale,
+                   QEMUTimerCB *cb, void *opaque)
+{
+    ts->timer_list = timer_list;
+    ts->cb = cb;
+    ts->opaque = opaque;
+    ts->scale = scale;
+    ts->expire_time = -1;
+}
+
+void timer_mod(QEMUTimer *ts, int64_t expire_time)
+{
+    QEMUTimerList *timer_list = ts->timer_list;
+    QEMUTimer *t = &timer_list->active_timers;
+
+    while (t->next != NULL) {
+        if (t->next == ts) {
+            break;
+        }
+
+        t = t->next;
+    }
+
+    ts->expire_time = MAX(expire_time * ts->scale, 0);
+    ts->next = NULL;
+    t->next = ts;
+}
+
+void timer_del(QEMUTimer *ts)
+{
+    QEMUTimerList *timer_list = ts->timer_list;
+    QEMUTimer *t = &timer_list->active_timers;
+
+    while (t->next != NULL) {
+        if (t->next == ts) {
+            t->next = ts->next;
+            return;
+        }
+
+        t = t->next;
+    }
+}
+
+int64_t qemu_clock_get_ns(QEMUClockType type)
+{
+    return ptimer_test_time_ns;
+}
+
+int64_t qemu_clock_deadline_ns_all(QEMUClockType type)
+{
+    QEMUTimerList *timer_list = main_loop_tlg.tl[type];
+    QEMUTimer *t = timer_list->active_timers.next;
+    int64_t deadline = -1;
+
+    while (t != NULL) {
+        if (deadline == -1) {
+            deadline = t->expire_time;
+        } else {
+            deadline = MIN(deadline, t->expire_time);
+        }
+
+        t = t->next;
+    }
+
+    return deadline;
+}
+
+QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
+{
+    QEMUBH *bh = g_new(QEMUBH, 1);
+
+    bh->cb = cb;
+    bh->opaque = opaque;
+
+    return bh;
+}
+
+void replay_bh_schedule_event(QEMUBH *bh)
+{
+    bh->cb(bh->opaque);
+}
diff --git a/tests/ptimer-test.c b/tests/ptimer-test.c
new file mode 100644
index 0000000..f207eeb
--- /dev/null
+++ b/tests/ptimer-test.c
@@ -0,0 +1,568 @@
+/*
+ * QTest testcase for the ptimer
+ *
+ * Author: Dmitry Osipenko <digetx@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <glib/gprintf.h>
+
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "hw/ptimer.h"
+
+#include "libqtest.h"
+#include "ptimer-test.h"
+
+static bool triggered;
+
+static void ptimer_trigger(void *opaque)
+{
+    triggered = true;
+}
+
+static void ptimer_test_expire_qemu_timers(int64_t expire_time,
+                                           QEMUClockType type)
+{
+    QEMUTimerList *timer_list = main_loop_tlg.tl[type];
+    QEMUTimer *t = timer_list->active_timers.next;
+
+    while (t != NULL) {
+        if (t->expire_time == expire_time) {
+            timer_del(t);
+
+            if (t->cb != NULL) {
+                t->cb(t->opaque);
+            }
+        }
+
+        t = t->next;
+    }
+}
+
+static void ptimer_test_set_qemu_time_ns(int64_t ns)
+{
+    ptimer_test_time_ns = ns;
+}
+
+static void qemu_clock_step(uint64_t ns)
+{
+    int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
+    int64_t advanced_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns;
+
+    while (deadline != -1 && deadline <= advanced_time) {
+        ptimer_test_set_qemu_time_ns(deadline);
+        ptimer_test_expire_qemu_timers(deadline, QEMU_CLOCK_VIRTUAL);
+        deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
+    }
+
+    ptimer_test_set_qemu_time_ns(advanced_time);
+}
+
+static void check_set_count(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_count(ptimer, 1000);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 1000);
+    g_assert_false(triggered);
+}
+
+static void check_set_limit(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_limit(ptimer, 1000, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_cmpuint(ptimer_get_limit(ptimer), ==, 1000);
+    g_assert_false(triggered);
+
+    ptimer_set_limit(ptimer, 2000, 1);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 2000);
+    g_assert_cmpuint(ptimer_get_limit(ptimer), ==, 2000);
+    g_assert_false(triggered);
+}
+
+static void check_oneshot(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_period(ptimer, 2000000);
+    ptimer_set_count(ptimer, 10);
+    ptimer_run(ptimer, 1);
+
+    qemu_clock_step(2000000 * 2 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_false(triggered);
+
+    ptimer_stop(ptimer);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000 * 11);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_false(triggered);
+
+    ptimer_run(ptimer, 1);
+
+    qemu_clock_step(2000000 * 7 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    qemu_clock_step(4000000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    ptimer_set_count(ptimer, 10);
+
+    qemu_clock_step(20000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 10);
+    g_assert_false(triggered);
+
+    ptimer_set_limit(ptimer, 9, 1);
+
+    qemu_clock_step(20000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 9);
+    g_assert_false(triggered);
+
+    ptimer_run(ptimer, 1);
+
+    qemu_clock_step(2000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_false(triggered);
+
+    ptimer_set_count(ptimer, 20);
+
+    qemu_clock_step(2000000 * 19 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+
+    ptimer_stop(ptimer);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 * 12 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+}
+
+static void check_periodic(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_period(ptimer, 2000000);
+    ptimer_set_limit(ptimer, 10, 1);
+    ptimer_run(ptimer, 0);
+
+    qemu_clock_step(2000000 * 10 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 9);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_false(triggered);
+
+    ptimer_set_count(ptimer, 20);
+
+    qemu_clock_step(2000000 * 11 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000 * 10);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_true(triggered);
+
+    ptimer_stop(ptimer);
+    triggered = false;
+
+    qemu_clock_step(2000000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_false(triggered);
+
+    ptimer_set_count(ptimer, 3);
+    ptimer_run(ptimer, 0);
+
+    qemu_clock_step(2000000 * 3 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 9);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_false(triggered);
+
+    ptimer_set_count(ptimer, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 10);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 * 12 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_true(triggered);
+
+    ptimer_stop(ptimer);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 * 12 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_false(triggered);
+
+    ptimer_run(ptimer, 0);
+    ptimer_set_period(ptimer, 0);
+
+    qemu_clock_step(2000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_false(triggered);
+}
+
+static void check_on_the_fly_mode_change(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_period(ptimer, 2000000);
+    ptimer_set_limit(ptimer, 10, 1);
+    ptimer_run(ptimer, 1);
+
+    qemu_clock_step(2000000 * 9 + 100000);
+
+    ptimer_run(ptimer, 0);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 9);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 * 9);
+
+    ptimer_run(ptimer, 1);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000 * 3);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+}
+
+static void check_on_the_fly_period_change(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_period(ptimer, 2000000);
+    ptimer_set_limit(ptimer, 8, 1);
+    ptimer_run(ptimer, 1);
+
+    qemu_clock_step(2000000 * 4 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+    g_assert_false(triggered);
+
+    ptimer_set_period(ptimer, 4000000);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+
+    qemu_clock_step(4000000 * 2 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    qemu_clock_step(4000000 * 2);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+}
+
+static void check_on_the_fly_freq_change(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_freq(ptimer, 500);
+    ptimer_set_limit(ptimer, 8, 1);
+    ptimer_run(ptimer, 1);
+
+    qemu_clock_step(2000000 * 4 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+    g_assert_false(triggered);
+
+    ptimer_set_freq(ptimer, 250);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+
+    qemu_clock_step(2000000 * 4 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000 * 4);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+}
+
+static void check_run_with_period_0(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_count(ptimer, 99);
+    ptimer_run(ptimer, 1);
+
+    qemu_clock_step(10 * NANOSECONDS_PER_SECOND);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 99);
+    g_assert_false(triggered);
+}
+
+static void check_run_with_delta_0(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_period(ptimer, 2000000);
+    ptimer_set_limit(ptimer, 99, 0);
+    ptimer_run(ptimer, 1);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 99);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 97);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000 * 97);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000 * 2);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    ptimer_set_count(ptimer, 0);
+    ptimer_run(ptimer, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 99);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 97);
+    g_assert_false(triggered);
+
+    qemu_clock_step(2000000 * 98);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 98);
+    g_assert_true(triggered);
+
+    ptimer_stop(ptimer);
+}
+
+static void check_periodic_with_load_0(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_period(ptimer, 2000000);
+    ptimer_run(ptimer, 0);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    ptimer_stop(ptimer);
+}
+
+static void check_oneshot_with_load_0(gconstpointer arg)
+{
+    const uint8_t *policy = arg;
+    QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
+    ptimer_state *ptimer = ptimer_init(bh, *policy);
+
+    triggered = false;
+
+    ptimer_set_period(ptimer, 2000000);
+    ptimer_run(ptimer, 1);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_true(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 + 100000);
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_false(triggered);
+
+    triggered = false;
+
+    qemu_clock_step(2000000 + 100000);
+
+    g_assert_false(triggered);
+}
+
+static void add_ptimer_tests(uint8_t policy)
+{
+    uint8_t *ppolicy = g_malloc(1);
+    char *policy_name = g_malloc(64);
+
+    *ppolicy = policy;
+
+    if (policy == PTIMER_POLICY_DEFAULT) {
+        g_sprintf(policy_name, "default");
+    }
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/set_count policy=%s", policy_name),
+        ppolicy, check_set_count);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/set_limit policy=%s", policy_name),
+        ppolicy, check_set_limit);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/oneshot policy=%s", policy_name),
+        ppolicy, check_oneshot);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/periodic policy=%s", policy_name),
+        ppolicy, check_periodic);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/on_the_fly_mode_change policy=%s", policy_name),
+        ppolicy, check_on_the_fly_mode_change);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/on_the_fly_period_change policy=%s", policy_name),
+        ppolicy, check_on_the_fly_period_change);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/on_the_fly_freq_change policy=%s", policy_name),
+        ppolicy, check_on_the_fly_freq_change);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/run_with_period_0 policy=%s", policy_name),
+        ppolicy, check_run_with_period_0);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/run_with_delta_0 policy=%s", policy_name),
+        ppolicy, check_run_with_delta_0);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/periodic_with_load_0 policy=%s", policy_name),
+        ppolicy, check_periodic_with_load_0);
+
+    qtest_add_data_func(
+        g_strdup_printf("/ptimer/oneshot_with_load_0 policy=%s", policy_name),
+        ppolicy, check_oneshot_with_load_0);
+}
+
+int main(int argc, char **argv)
+{
+    int i;
+
+    g_test_init(&argc, &argv, NULL);
+
+    for (i = 0; i < QEMU_CLOCK_MAX; i++) {
+        main_loop_tlg.tl[i] = g_new0(QEMUTimerList, 1);
+    }
+
+    add_ptimer_tests(PTIMER_POLICY_DEFAULT);
+
+    qtest_allowed = true;
+
+    return g_test_run();
+}
diff --git a/tests/ptimer-test.h b/tests/ptimer-test.h
new file mode 100644
index 0000000..98d9b8f
--- /dev/null
+++ b/tests/ptimer-test.h
@@ -0,0 +1,22 @@
+/*
+ * QTest testcase for the ptimer
+ *
+ * Author: Dmitry Osipenko <digetx@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef PTIMER_TEST_H
+#define PTIMER_TEST_H
+
+extern bool qtest_allowed;
+
+extern int64_t ptimer_test_time_ns;
+
+struct QEMUTimerList {
+    QEMUTimer active_timers;
+};
+
+#endif
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 04/16] hw/ptimer: Suppress error messages under qtest
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 05/16] hw/ptimer: Add "wraparound after one period" policy Dmitry Osipenko
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Under qtest ptimer emits lots of warning messages. The messages are caused
by the actual checking of the ptimer error conditions. Suppress those
messages, so they do not distract.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 00fdf15..c45c835 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -11,6 +11,7 @@
 #include "hw/ptimer.h"
 #include "qemu/host-utils.h"
 #include "sysemu/replay.h"
+#include "sysemu/qtest.h"
 
 struct ptimer_state
 {
@@ -44,7 +45,9 @@ static void ptimer_reload(ptimer_state *s)
         s->delta = s->limit;
     }
     if (s->delta == 0 || s->period == 0) {
-        fprintf(stderr, "Timer with period zero, disabling\n");
+        if (!qtest_enabled()) {
+            fprintf(stderr, "Timer with period zero, disabling\n");
+        }
         timer_del(s->timer);
         s->enabled = 0;
         return;
@@ -163,7 +166,9 @@ void ptimer_run(ptimer_state *s, int oneshot)
     bool was_disabled = !s->enabled;
 
     if (was_disabled && s->period == 0) {
-        fprintf(stderr, "Timer with period zero, disabling\n");
+        if (!qtest_enabled()) {
+            fprintf(stderr, "Timer with period zero, disabling\n");
+        }
         return;
     }
     s->enabled = oneshot ? 2 : 1;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 05/16] hw/ptimer: Add "wraparound after one period" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 04/16] hw/ptimer: Suppress error messages under qtest Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-20 17:20   ` Peter Maydell
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 06/16] tests: ptimer: Add tests for " Dmitry Osipenko
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Currently, periodic counter wraps around immediately once counter reaches
"0", this is wrong behaviour for some of the timers, resulting in one period
being lost. Add new ptimer policy that provides correct behaviour for such
timers, so that counter stays with "0" for a one period before wrapping
around.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c    | 43 ++++++++++++++++++++++++++++---------------
 include/hw/ptimer.h |  4 ++++
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index c45c835..e9b2e15 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -35,16 +35,17 @@ static void ptimer_trigger(ptimer_state *s)
     }
 }
 
-static void ptimer_reload(ptimer_state *s)
+static void ptimer_reload(ptimer_state *s, int delta_adjust)
 {
     uint32_t period_frac = s->period_frac;
     uint64_t period = s->period;
+    uint64_t delta = s->delta;
 
-    if (s->delta == 0) {
+    if (delta == 0) {
         ptimer_trigger(s);
-        s->delta = s->limit;
+        delta = s->delta = s->limit;
     }
-    if (s->delta == 0 || s->period == 0) {
+    if (delta == 0 || s->period == 0) {
         if (!qtest_enabled()) {
             fprintf(stderr, "Timer with period zero, disabling\n");
         }
@@ -53,6 +54,10 @@ static void ptimer_reload(ptimer_state *s)
         return;
     }
 
+    if (s->policy_mask & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD) {
+        delta += delta_adjust;
+    }
+
     /*
      * Artificially limit timeout rate to something
      * achievable under QEMU.  Otherwise, QEMU spends all
@@ -62,15 +67,15 @@ static void ptimer_reload(ptimer_state *s)
      * on the current generation of host machines.
      */
 
-    if (s->enabled == 1 && (s->delta * period < 10000) && !use_icount) {
-        period = 10000 / s->delta;
+    if (s->enabled == 1 && (delta * period < 10000) && !use_icount) {
+        period = 10000 / delta;
         period_frac = 0;
     }
 
     s->last_event = s->next_event;
-    s->next_event = s->last_event + s->delta * period;
+    s->next_event = s->last_event + delta * period;
     if (period_frac) {
-        s->next_event += ((int64_t)period_frac * s->delta) >> 32;
+        s->next_event += ((int64_t)period_frac * delta) >> 32;
     }
     timer_mod(s->timer, s->next_event);
 }
@@ -83,7 +88,7 @@ static void ptimer_tick(void *opaque)
     if (s->enabled == 2) {
         s->enabled = 0;
     } else {
-        ptimer_reload(s);
+        ptimer_reload(s, 1);
     }
 }
 
@@ -91,7 +96,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
 {
     uint64_t counter;
 
-    if (s->enabled) {
+    if (s->enabled && s->delta != 0) {
         int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
         int64_t next = s->next_event;
         bool expired = (now - next >= 0);
@@ -145,6 +150,14 @@ uint64_t ptimer_get_count(ptimer_state *s)
                     div += 1;
             }
             counter = rem / div;
+
+            if (!oneshot && s->delta == s->limit) {
+                /* Before wrapping around, timer should stay with counter = 0
+                   for a one period.  The delta has been adjusted by +1 for
+                   the wrapped around counter, so taking a modulo of limit + 1
+                   gives that period.  */
+                counter %= s->limit + 1;
+            }
         }
     } else {
         counter = s->delta;
@@ -157,7 +170,7 @@ void ptimer_set_count(ptimer_state *s, uint64_t count)
     s->delta = count;
     if (s->enabled) {
         s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        ptimer_reload(s);
+        ptimer_reload(s, 0);
     }
 }
 
@@ -174,7 +187,7 @@ void ptimer_run(ptimer_state *s, int oneshot)
     s->enabled = oneshot ? 2 : 1;
     if (was_disabled) {
         s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        ptimer_reload(s);
+        ptimer_reload(s, 0);
     }
 }
 
@@ -198,7 +211,7 @@ void ptimer_set_period(ptimer_state *s, int64_t period)
     s->period_frac = 0;
     if (s->enabled) {
         s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        ptimer_reload(s);
+        ptimer_reload(s, 0);
     }
 }
 
@@ -210,7 +223,7 @@ void ptimer_set_freq(ptimer_state *s, uint32_t freq)
     s->period_frac = (1000000000ll << 32) / freq;
     if (s->enabled) {
         s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        ptimer_reload(s);
+        ptimer_reload(s, 0);
     }
 }
 
@@ -223,7 +236,7 @@ void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
         s->delta = limit;
     if (s->enabled && reload) {
         s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        ptimer_reload(s);
+        ptimer_reload(s, 0);
     }
 }
 
diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index 7b0fc13..a13a12e 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -34,6 +34,10 @@
  *    counter = counter value at the moment of change (.i.e. one less).  */
 #define PTIMER_POLICY_DEFAULT               0
 
+/* Periodic timer counter stays with "0" for a one period before wrapping
+ * around.  */
+#define PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD (1 << 0)
+
 /* ptimer.c */
 typedef struct ptimer_state ptimer_state;
 typedef void (*ptimer_cb)(void *opaque);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 06/16] tests: ptimer: Add tests for "wraparound after one period" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 05/16] hw/ptimer: Add "wraparound after one period" policy Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy Dmitry Osipenko
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD changes ptimer behaviour in a such way,
that it would wrap around after one period instead of doing it immediately.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 tests/ptimer-test.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/tests/ptimer-test.c b/tests/ptimer-test.c
index f207eeb..5b0d75b 100644
--- a/tests/ptimer-test.c
+++ b/tests/ptimer-test.c
@@ -188,6 +188,7 @@ static void check_periodic(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
 
     triggered = false;
 
@@ -197,14 +198,14 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 10 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 9);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 10 : 9);
     g_assert_true(triggered);
 
     triggered = false;
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 20);
@@ -216,7 +217,7 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 10);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
     g_assert_true(triggered);
 
     ptimer_stop(ptimer);
@@ -224,7 +225,7 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 3);
@@ -232,14 +233,14 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 3 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 9);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 10 : 9);
     g_assert_true(triggered);
 
     triggered = false;
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 0);
@@ -250,7 +251,7 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 12 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 8 : 7);
     g_assert_true(triggered);
 
     ptimer_stop(ptimer);
@@ -259,7 +260,7 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 12 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 8 : 7);
     g_assert_false(triggered);
 
     ptimer_run(ptimer, 0);
@@ -267,7 +268,7 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 8 : 7);
     g_assert_false(triggered);
 }
 
@@ -276,6 +277,7 @@ static void check_on_the_fly_mode_change(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
 
     triggered = false;
 
@@ -292,7 +294,7 @@ static void check_on_the_fly_mode_change(gconstpointer arg)
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 9);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 10 : 9);
     g_assert_true(triggered);
 
     triggered = false;
@@ -301,7 +303,7 @@ static void check_on_the_fly_mode_change(gconstpointer arg)
 
     ptimer_run(ptimer, 1);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 1 : 0);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 3);
@@ -394,6 +396,7 @@ static void check_run_with_delta_0(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
 
     triggered = false;
 
@@ -436,7 +439,7 @@ static void check_run_with_delta_0(gconstpointer arg)
 
     qemu_clock_step(2000000 * 98);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 98);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 99 : 98);
     g_assert_true(triggered);
 
     ptimer_stop(ptimer);
@@ -497,7 +500,7 @@ static void check_oneshot_with_load_0(gconstpointer arg)
 static void add_ptimer_tests(uint8_t policy)
 {
     uint8_t *ppolicy = g_malloc(1);
-    char *policy_name = g_malloc(64);
+    char *policy_name = g_malloc0(256);
 
     *ppolicy = policy;
 
@@ -505,6 +508,10 @@ static void add_ptimer_tests(uint8_t policy)
         g_sprintf(policy_name, "default");
     }
 
+    if (policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD) {
+        g_strlcat(policy_name, "wrap_after_one_period,", 256);
+    }
+
     qtest_add_data_func(
         g_strdup_printf("/ptimer/set_count policy=%s", policy_name),
         ppolicy, check_set_count);
@@ -550,6 +557,16 @@ static void add_ptimer_tests(uint8_t policy)
         ppolicy, check_oneshot_with_load_0);
 }
 
+static void add_all_ptimer_policies_comb_tests(void)
+{
+    int last_policy = PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD;
+    int policy = PTIMER_POLICY_DEFAULT;
+
+    for (; policy < (last_policy << 1); policy++) {
+        add_ptimer_tests(policy);
+    }
+}
+
 int main(int argc, char **argv)
 {
     int i;
@@ -560,7 +577,7 @@ int main(int argc, char **argv)
         main_loop_tlg.tl[i] = g_new0(QEMUTimerList, 1);
     }
 
-    add_ptimer_tests(PTIMER_POLICY_DEFAULT);
+    add_all_ptimer_policies_comb_tests();
 
     qtest_allowed = true;
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 06/16] tests: ptimer: Add tests for " Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-20 17:21   ` Peter Maydell
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 08/16] tests: ptimer: Add tests for " Dmitry Osipenko
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Currently, periodic timer that has load = delta = 0 performs trigger
on timer reload and stops, printing a "period zero" error message.
Introduce new policy that makes periodic timer to continuously trigger
with a period interval in case of load = 0.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c    | 18 +++++++++++++++++-
 include/hw/ptimer.h |  4 ++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index e9b2e15..97ce8ae 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -45,7 +45,8 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
         ptimer_trigger(s);
         delta = s->delta = s->limit;
     }
-    if (delta == 0 || s->period == 0) {
+
+    if (s->period == 0) {
         if (!qtest_enabled()) {
             fprintf(stderr, "Timer with period zero, disabling\n");
         }
@@ -58,6 +59,21 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
         delta += delta_adjust;
     }
 
+    if (delta == 0 && (s->policy_mask & PTIMER_POLICY_CONTINUOUS_TRIGGER)) {
+        if (s->enabled == 1) {
+            delta = 1;
+        }
+    }
+
+    if (delta == 0) {
+        if (!qtest_enabled()) {
+            fprintf(stderr, "Timer with delta zero, disabling\n");
+        }
+        timer_del(s->timer);
+        s->enabled = 0;
+        return;
+    }
+
     /*
      * Artificially limit timeout rate to something
      * achievable under QEMU.  Otherwise, QEMU spends all
diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index a13a12e..4c9c700 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -38,6 +38,10 @@
  * around.  */
 #define PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD (1 << 0)
 
+/* Periodic timer that has load = 0 would continuously re-trigger every
+ * period.  */
+#define PTIMER_POLICY_CONTINUOUS_TRIGGER    (1 << 1)
+
 /* ptimer.c */
 typedef struct ptimer_state ptimer_state;
 typedef void (*ptimer_cb)(void *opaque);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 08/16] tests: ptimer: Add tests for "continuous trigger" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (6 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 09/16] hw/ptimer: Add "no immediate " Dmitry Osipenko
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

PTIMER_POLICY_CONTINUOUS_TRIGGER makes periodic ptimer to re-trigger every
period in case of load = delta = 0.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 include/hw/ptimer.h |  4 ++--
 tests/ptimer-test.c | 14 ++++++++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index 4c9c700..e2f4c55 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -38,8 +38,8 @@
  * around.  */
 #define PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD (1 << 0)
 
-/* Periodic timer that has load = 0 would continuously re-trigger every
- * period.  */
+/* Running periodic timer that has counter = limit = 0 would continuously
+ * re-trigger every period.  */
 #define PTIMER_POLICY_CONTINUOUS_TRIGGER    (1 << 1)
 
 /* ptimer.c */
diff --git a/tests/ptimer-test.c b/tests/ptimer-test.c
index 5b0d75b..30a2ae1 100644
--- a/tests/ptimer-test.c
+++ b/tests/ptimer-test.c
@@ -450,6 +450,7 @@ static void check_periodic_with_load_0(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool continuous_trigger = (*policy & PTIMER_POLICY_CONTINUOUS_TRIGGER);
 
     triggered = false;
 
@@ -464,7 +465,12 @@ static void check_periodic_with_load_0(gconstpointer arg)
     qemu_clock_step(2000000 + 100000);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
-    g_assert_false(triggered);
+
+    if (continuous_trigger) {
+        g_assert_true(triggered);
+    } else {
+        g_assert_false(triggered);
+    }
 
     ptimer_stop(ptimer);
 }
@@ -512,6 +518,10 @@ static void add_ptimer_tests(uint8_t policy)
         g_strlcat(policy_name, "wrap_after_one_period,", 256);
     }
 
+    if (policy & PTIMER_POLICY_CONTINUOUS_TRIGGER) {
+        g_strlcat(policy_name, "continuous_trigger,", 256);
+    }
+
     qtest_add_data_func(
         g_strdup_printf("/ptimer/set_count policy=%s", policy_name),
         ppolicy, check_set_count);
@@ -559,7 +569,7 @@ static void add_ptimer_tests(uint8_t policy)
 
 static void add_all_ptimer_policies_comb_tests(void)
 {
-    int last_policy = PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD;
+    int last_policy = PTIMER_POLICY_CONTINUOUS_TRIGGER;
     int policy = PTIMER_POLICY_DEFAULT;
 
     for (; policy < (last_policy << 1); policy++) {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 09/16] hw/ptimer: Add "no immediate trigger" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (7 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 08/16] tests: ptimer: Add tests for " Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 10/16] tests: ptimer: Add tests for " Dmitry Osipenko
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Performing trigger on setting (or starting to run with) counter = 0 could
be a wrong behaviour for some of the timers, provide "no immediate trigger"
policy to maintain correct behaviour for such timers.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c    | 9 ++++++++-
 include/hw/ptimer.h | 4 ++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 97ce8ae..018589d 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -41,8 +41,11 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
     uint64_t period = s->period;
     uint64_t delta = s->delta;
 
-    if (delta == 0) {
+    if (delta == 0 && !(s->policy_mask & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER)) {
         ptimer_trigger(s);
+    }
+
+    if (delta == 0) {
         delta = s->delta = s->limit;
     }
 
@@ -65,6 +68,10 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
         }
     }
 
+    if (delta == 0 && (s->policy_mask & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER)) {
+        delta = 1;
+    }
+
     if (delta == 0) {
         if (!qtest_enabled()) {
             fprintf(stderr, "Timer with delta zero, disabling\n");
diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index e2f4c55..4b7c53c 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -42,6 +42,10 @@
  * re-trigger every period.  */
 #define PTIMER_POLICY_CONTINUOUS_TRIGGER    (1 << 1)
 
+/* Starting to run with/setting counter = 0 won't perform immediate
+ * trigger.  */
+#define PTIMER_POLICY_NO_IMMEDIATE_TRIGGER  (1 << 2)
+
 /* ptimer.c */
 typedef struct ptimer_state ptimer_state;
 typedef void (*ptimer_cb)(void *opaque);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 10/16] tests: ptimer: Add tests for "no immediate trigger" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (8 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 09/16] hw/ptimer: Add "no immediate " Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 11/16] hw/ptimer: Add "no immediate reload" policy Dmitry Osipenko
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

PTIMER_POLICY_NO_IMMEDIATE_TRIGGER makes ptimer to not to trigger on starting
to run with / setting counter to "0".

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 tests/ptimer-test.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/tests/ptimer-test.c b/tests/ptimer-test.c
index 30a2ae1..3e44b8d 100644
--- a/tests/ptimer-test.c
+++ b/tests/ptimer-test.c
@@ -189,6 +189,7 @@ static void check_periodic(gconstpointer arg)
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
     bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
+    bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
 
     triggered = false;
 
@@ -245,7 +246,12 @@ static void check_periodic(gconstpointer arg)
 
     ptimer_set_count(ptimer, 0);
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 10);
-    g_assert_true(triggered);
+
+    if (no_immediate_trigger) {
+        g_assert_false(triggered);
+    } else {
+        g_assert_true(triggered);
+    }
 
     triggered = false;
 
@@ -397,6 +403,7 @@ static void check_run_with_delta_0(gconstpointer arg)
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
     bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
+    bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
 
     triggered = false;
 
@@ -404,7 +411,12 @@ static void check_run_with_delta_0(gconstpointer arg)
     ptimer_set_limit(ptimer, 99, 0);
     ptimer_run(ptimer, 1);
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 99);
-    g_assert_true(triggered);
+
+    if (no_immediate_trigger) {
+        g_assert_false(triggered);
+    } else {
+        g_assert_true(triggered);
+    }
 
     triggered = false;
 
@@ -428,7 +440,12 @@ static void check_run_with_delta_0(gconstpointer arg)
     ptimer_set_count(ptimer, 0);
     ptimer_run(ptimer, 0);
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 99);
-    g_assert_true(triggered);
+
+    if (no_immediate_trigger) {
+        g_assert_false(triggered);
+    } else {
+        g_assert_true(triggered);
+    }
 
     triggered = false;
 
@@ -451,6 +468,7 @@ static void check_periodic_with_load_0(gconstpointer arg)
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
     bool continuous_trigger = (*policy & PTIMER_POLICY_CONTINUOUS_TRIGGER);
+    bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
 
     triggered = false;
 
@@ -458,7 +476,12 @@ static void check_periodic_with_load_0(gconstpointer arg)
     ptimer_run(ptimer, 0);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
-    g_assert_true(triggered);
+
+    if (no_immediate_trigger) {
+        g_assert_false(triggered);
+    } else {
+        g_assert_true(triggered);
+    }
 
     triggered = false;
 
@@ -466,7 +489,7 @@ static void check_periodic_with_load_0(gconstpointer arg)
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
 
-    if (continuous_trigger) {
+    if (continuous_trigger || no_immediate_trigger) {
         g_assert_true(triggered);
     } else {
         g_assert_false(triggered);
@@ -480,6 +503,7 @@ static void check_oneshot_with_load_0(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
 
     triggered = false;
 
@@ -487,13 +511,29 @@ static void check_oneshot_with_load_0(gconstpointer arg)
     ptimer_run(ptimer, 1);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
-    g_assert_true(triggered);
+
+    if (no_immediate_trigger) {
+        g_assert_false(triggered);
+    } else {
+        g_assert_true(triggered);
+    }
 
     triggered = false;
 
     qemu_clock_step(2000000 + 100000);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+
+    if (no_immediate_trigger) {
+        g_assert_true(triggered);
+    } else {
+        g_assert_false(triggered);
+    }
+
+    triggered = false;
+
+    qemu_clock_step(2000000 + 100000);
+
     g_assert_false(triggered);
 
     triggered = false;
@@ -522,6 +562,10 @@ static void add_ptimer_tests(uint8_t policy)
         g_strlcat(policy_name, "continuous_trigger,", 256);
     }
 
+    if (policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER) {
+        g_strlcat(policy_name, "no_immediate_trigger,", 256);
+    }
+
     qtest_add_data_func(
         g_strdup_printf("/ptimer/set_count policy=%s", policy_name),
         ppolicy, check_set_count);
@@ -569,7 +613,7 @@ static void add_ptimer_tests(uint8_t policy)
 
 static void add_all_ptimer_policies_comb_tests(void)
 {
-    int last_policy = PTIMER_POLICY_CONTINUOUS_TRIGGER;
+    int last_policy = PTIMER_POLICY_NO_IMMEDIATE_TRIGGER;
     int policy = PTIMER_POLICY_DEFAULT;
 
     for (; policy < (last_policy << 1); policy++) {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 11/16] hw/ptimer: Add "no immediate reload" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (9 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 10/16] tests: ptimer: Add tests for " Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 12/16] tests: ptimer: Add tests for " Dmitry Osipenko
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Immediate counter re-load on setting (or on starting to run with)
counter = 0 is a wrong behaviour for some of the timers. Add "no
immediate reload" policy that provides correct behaviour for such timers.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c    | 8 +++++---
 include/hw/ptimer.h | 3 +++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 018589d..f7f5538 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -45,7 +45,7 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
         ptimer_trigger(s);
     }
 
-    if (delta == 0) {
+    if (delta == 0 && !(s->policy_mask & PTIMER_POLICY_NO_IMMEDIATE_RELOAD)) {
         delta = s->delta = s->limit;
     }
 
@@ -107,11 +107,13 @@ static void ptimer_tick(void *opaque)
 {
     ptimer_state *s = (ptimer_state *)opaque;
     ptimer_trigger(s);
-    s->delta = 0;
     if (s->enabled == 2) {
+        s->delta = 0;
         s->enabled = 0;
     } else {
-        ptimer_reload(s, 1);
+        int delta_adjust = (s->delta != 0 && s->limit != 0) ? 1 : 0;
+        s->delta = s->limit;
+        ptimer_reload(s, delta_adjust);
     }
 }
 
diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index 4b7c53c..131fed1 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -46,6 +46,9 @@
  * trigger.  */
 #define PTIMER_POLICY_NO_IMMEDIATE_TRIGGER  (1 << 2)
 
+/* Starting to run with/setting counter = 0 won't re-load counter.  */
+#define PTIMER_POLICY_NO_IMMEDIATE_RELOAD   (1 << 3)
+
 /* ptimer.c */
 typedef struct ptimer_state ptimer_state;
 typedef void (*ptimer_cb)(void *opaque);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 12/16] tests: ptimer: Add tests for "no immediate reload" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (10 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 11/16] hw/ptimer: Add "no immediate reload" policy Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 13/16] hw/ptimer: Add "no counter round down" policy Dmitry Osipenko
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

PTIMER_POLICY_NO_IMMEDIATE_RELOAD makes ptimer to not to re-load
counter on setting counter value to "0" or starting to run with "0".

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 tests/ptimer-test.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/tests/ptimer-test.c b/tests/ptimer-test.c
index 3e44b8d..bb49670 100644
--- a/tests/ptimer-test.c
+++ b/tests/ptimer-test.c
@@ -190,6 +190,8 @@ static void check_periodic(gconstpointer arg)
     ptimer_state *ptimer = ptimer_init(bh, *policy);
     bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
     bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
+    bool no_immediate_reload = (*policy & PTIMER_POLICY_NO_IMMEDIATE_RELOAD);
+    bool continuous_trigger = (*policy & PTIMER_POLICY_CONTINUOUS_TRIGGER);
 
     triggered = false;
 
@@ -245,7 +247,7 @@ static void check_periodic(gconstpointer arg)
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 0);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 10);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_immediate_reload ? 0 : 10);
 
     if (no_immediate_trigger) {
         g_assert_false(triggered);
@@ -257,7 +259,14 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 12 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 8 : 7);
+    if (no_immediate_reload && (!continuous_trigger && !no_immediate_trigger)) {
+        g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+        g_assert_false(triggered);
+        return;
+    }
+
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                    7 + (wrap_policy ? 1 : 0) + (no_immediate_reload ? 1 : 0));
     g_assert_true(triggered);
 
     ptimer_stop(ptimer);
@@ -266,7 +275,8 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 12 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 8 : 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                    7 + (wrap_policy ? 1 : 0) + (no_immediate_reload ? 1 : 0));
     g_assert_false(triggered);
 
     ptimer_run(ptimer, 0);
@@ -274,7 +284,8 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 8 : 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                    7 + (wrap_policy ? 1 : 0) + (no_immediate_reload ? 1 : 0));
     g_assert_false(triggered);
 }
 
@@ -404,13 +415,15 @@ static void check_run_with_delta_0(gconstpointer arg)
     ptimer_state *ptimer = ptimer_init(bh, *policy);
     bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
     bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
+    bool no_immediate_reload = (*policy & PTIMER_POLICY_NO_IMMEDIATE_RELOAD);
+    bool continuous_trigger = (*policy & PTIMER_POLICY_CONTINUOUS_TRIGGER);
 
     triggered = false;
 
     ptimer_set_period(ptimer, 2000000);
     ptimer_set_limit(ptimer, 99, 0);
     ptimer_run(ptimer, 1);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 99);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_immediate_reload ? 0 : 99);
 
     if (no_immediate_trigger) {
         g_assert_false(triggered);
@@ -420,6 +433,11 @@ static void check_run_with_delta_0(gconstpointer arg)
 
     triggered = false;
 
+    if (no_immediate_reload) {
+        ptimer_set_count(ptimer, 99);
+        ptimer_run(ptimer, 1);
+    }
+
     qemu_clock_step(2000000 + 100000);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 97);
@@ -439,7 +457,7 @@ static void check_run_with_delta_0(gconstpointer arg)
 
     ptimer_set_count(ptimer, 0);
     ptimer_run(ptimer, 0);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 99);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_immediate_reload ? 0 : 99);
 
     if (no_immediate_trigger) {
         g_assert_false(triggered);
@@ -449,6 +467,21 @@ static void check_run_with_delta_0(gconstpointer arg)
 
     triggered = false;
 
+    if (no_immediate_reload) {
+        qemu_clock_step(2000000 + 100000);
+
+        if (continuous_trigger || no_immediate_trigger) {
+            g_assert_cmpuint(ptimer_get_count(ptimer), ==, 98);
+            g_assert_true(triggered);
+
+            triggered = false;
+        } else {
+            g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+            g_assert_false(triggered);
+            return;
+        }
+    }
+
     qemu_clock_step(2000000 + 100000);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 97);
@@ -566,6 +599,10 @@ static void add_ptimer_tests(uint8_t policy)
         g_strlcat(policy_name, "no_immediate_trigger,", 256);
     }
 
+    if (policy & PTIMER_POLICY_NO_IMMEDIATE_RELOAD) {
+        g_strlcat(policy_name, "no_immediate_reload,", 256);
+    }
+
     qtest_add_data_func(
         g_strdup_printf("/ptimer/set_count policy=%s", policy_name),
         ppolicy, check_set_count);
@@ -613,7 +650,7 @@ static void add_ptimer_tests(uint8_t policy)
 
 static void add_all_ptimer_policies_comb_tests(void)
 {
-    int last_policy = PTIMER_POLICY_NO_IMMEDIATE_TRIGGER;
+    int last_policy = PTIMER_POLICY_NO_IMMEDIATE_RELOAD;
     int policy = PTIMER_POLICY_DEFAULT;
 
     for (; policy < (last_policy << 1); policy++) {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 13/16] hw/ptimer: Add "no counter round down" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (11 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 12/16] tests: ptimer: Add tests for " Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 14/16] tests: ptimer: Add tests for " Dmitry Osipenko
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

For most of the timers counter starts to decrement after first period
expires. Due to rounding down performed by the ptimer_get_count, it returns
counter - 1 for the running timer, so that for the ptimer user it looks
like counter gets decremented immediately after running the timer. Add "no
counter round down" policy that provides correct behaviour for those timers.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c    | 9 +++++++--
 include/hw/ptimer.h | 4 ++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index f7f5538..88a1fe2 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -119,11 +119,14 @@ static void ptimer_tick(void *opaque)
 
 uint64_t ptimer_get_count(ptimer_state *s)
 {
+    bool no_round_down =
+                !!(s->policy_mask & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
     uint64_t counter;
 
     if (s->enabled && s->delta != 0) {
         int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
         int64_t next = s->next_event;
+        int64_t last = s->last_event;
         bool expired = (now - next >= 0);
         bool oneshot = (s->enabled == 2);
 
@@ -131,7 +134,9 @@ uint64_t ptimer_get_count(ptimer_state *s)
         if (expired) {
             /* Prevent timer underflowing if it should already have
                triggered.  */
-            counter = 0;
+            counter = no_round_down ? 1 : 0;
+        } else if (now == last && no_round_down) {
+            counter = s->delta;
         } else {
             uint64_t rem;
             uint64_t div;
@@ -174,7 +179,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
                 if ((uint32_t)(period_frac << shift))
                     div += 1;
             }
-            counter = rem / div;
+            counter = rem / div + (no_round_down ? 1 : 0);
 
             if (!oneshot && s->delta == s->limit) {
                 /* Before wrapping around, timer should stay with counter = 0
diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index 131fed1..5ce3270 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -49,6 +49,10 @@
 /* Starting to run with/setting counter = 0 won't re-load counter.  */
 #define PTIMER_POLICY_NO_IMMEDIATE_RELOAD   (1 << 3)
 
+/* Make counter value of the running timer represent the actual value and
+ * not the one less.  */
+#define PTIMER_POLICY_NO_COUNTER_ROUND_DOWN (1 << 4)
+
 /* ptimer.c */
 typedef struct ptimer_state ptimer_state;
 typedef void (*ptimer_cb)(void *opaque);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 14/16] tests: ptimer: Add tests for "no counter round down" policy
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (12 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 13/16] hw/ptimer: Add "no counter round down" policy Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 15/16] arm_mptimer: Convert to use ptimer Dmitry Osipenko
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

PTIMER_POLICY_NO_COUNTER_ROUND_DOWN makes ptimer_get_count() return the
actual counter value and not the one less.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 tests/ptimer-test.c | 115 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 78 insertions(+), 37 deletions(-)

diff --git a/tests/ptimer-test.c b/tests/ptimer-test.c
index bb49670..a69e759 100644
--- a/tests/ptimer-test.c
+++ b/tests/ptimer-test.c
@@ -99,6 +99,7 @@ static void check_oneshot(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool no_round_down = (*policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
 
     triggered = false;
 
@@ -108,32 +109,44 @@ static void check_oneshot(gconstpointer arg)
 
     qemu_clock_step(2000000 * 2 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 8 : 7);
     g_assert_false(triggered);
 
     ptimer_stop(ptimer);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 8 : 7);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 11);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 8 : 7);
     g_assert_false(triggered);
 
     ptimer_run(ptimer, 1);
 
     qemu_clock_step(2000000 * 7 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
-    g_assert_true(triggered);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 1 : 0);
 
-    triggered = false;
+    if (no_round_down) {
+        g_assert_false(triggered);
+    } else {
+        g_assert_true(triggered);
+
+        triggered = false;
+    }
 
     qemu_clock_step(2000000);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
-    g_assert_false(triggered);
+
+    if (no_round_down) {
+        g_assert_true(triggered);
+
+        triggered = false;
+    } else {
+        g_assert_false(triggered);
+    }
 
     qemu_clock_step(4000000);
 
@@ -158,14 +171,14 @@ static void check_oneshot(gconstpointer arg)
 
     qemu_clock_step(2000000 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 7);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 8 : 7);
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 20);
 
     qemu_clock_step(2000000 * 19 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 1 : 0);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000);
@@ -192,6 +205,7 @@ static void check_periodic(gconstpointer arg)
     bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
     bool no_immediate_reload = (*policy & PTIMER_POLICY_NO_IMMEDIATE_RELOAD);
     bool continuous_trigger = (*policy & PTIMER_POLICY_CONTINUOUS_TRIGGER);
+    bool no_round_down = (*policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
 
     triggered = false;
 
@@ -201,26 +215,29 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 10 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 10 : 9);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+            wrap_policy ? (no_round_down ? 0 : 10) : (no_round_down ? 10 : 9));
     g_assert_true(triggered);
 
     triggered = false;
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     (no_round_down ? 1 : 0) + (wrap_policy ? 9 : 8));
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 20);
 
     qemu_clock_step(2000000 * 11 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 9 : 8);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 10);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     (no_round_down ? 1 : 0) + (wrap_policy ? 9 : 8));
     g_assert_true(triggered);
 
     ptimer_stop(ptimer);
@@ -228,7 +245,8 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     (no_round_down ? 1 : 0) + (wrap_policy ? 9 : 8));
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 3);
@@ -236,18 +254,21 @@ static void check_periodic(gconstpointer arg)
 
     qemu_clock_step(2000000 * 3 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 10 : 9);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+            wrap_policy ? (no_round_down ? 0 : 10) : (no_round_down ? 10 : 9));
     g_assert_true(triggered);
 
     triggered = false;
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 9 : 8);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     (no_round_down ? 1 : 0) + (wrap_policy ? 9 : 8));
     g_assert_false(triggered);
 
     ptimer_set_count(ptimer, 0);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_immediate_reload ? 0 : 10);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     no_immediate_reload ? 0 : 10);
 
     if (no_immediate_trigger) {
         g_assert_false(triggered);
@@ -266,7 +287,9 @@ static void check_periodic(gconstpointer arg)
     }
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==,
-                    7 + (wrap_policy ? 1 : 0) + (no_immediate_reload ? 1 : 0));
+                    (no_immediate_reload ? 8 : 7) +
+                    (no_round_down ? 1 : 0) +
+                    (wrap_policy ? 1 : 0));
     g_assert_true(triggered);
 
     ptimer_stop(ptimer);
@@ -276,7 +299,9 @@ static void check_periodic(gconstpointer arg)
     qemu_clock_step(2000000 * 12 + 100000);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==,
-                    7 + (wrap_policy ? 1 : 0) + (no_immediate_reload ? 1 : 0));
+                    (no_immediate_reload ? 8 : 7) +
+                    (no_round_down ? 1 : 0) +
+                    (wrap_policy ? 1 : 0));
     g_assert_false(triggered);
 
     ptimer_run(ptimer, 0);
@@ -285,7 +310,9 @@ static void check_periodic(gconstpointer arg)
     qemu_clock_step(2000000 + 100000);
 
     g_assert_cmpuint(ptimer_get_count(ptimer), ==,
-                    7 + (wrap_policy ? 1 : 0) + (no_immediate_reload ? 1 : 0));
+                    (no_immediate_reload ? 8 : 7) +
+                    (no_round_down ? 1 : 0) +
+                    (wrap_policy ? 1 : 0));
     g_assert_false(triggered);
 }
 
@@ -295,6 +322,7 @@ static void check_on_the_fly_mode_change(gconstpointer arg)
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
     bool wrap_policy = (*policy & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD);
+    bool no_round_down = (*policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
 
     triggered = false;
 
@@ -306,12 +334,13 @@ static void check_on_the_fly_mode_change(gconstpointer arg)
 
     ptimer_run(ptimer, 0);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 1 : 0);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 10 : 9);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+            wrap_policy ? (no_round_down ? 0 : 10) : (no_round_down ? 10 : 9));
     g_assert_true(triggered);
 
     triggered = false;
@@ -320,7 +349,8 @@ static void check_on_the_fly_mode_change(gconstpointer arg)
 
     ptimer_run(ptimer, 1);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 1 : 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     (no_round_down ? 1 : 0) + (wrap_policy ? 1 : 0));
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 3);
@@ -334,6 +364,7 @@ static void check_on_the_fly_period_change(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool no_round_down = (*policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
 
     triggered = false;
 
@@ -343,15 +374,15 @@ static void check_on_the_fly_period_change(gconstpointer arg)
 
     qemu_clock_step(2000000 * 4 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 4 : 3);
     g_assert_false(triggered);
 
     ptimer_set_period(ptimer, 4000000);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 4 : 3);
 
     qemu_clock_step(4000000 * 2 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 2 : 0);
     g_assert_false(triggered);
 
     qemu_clock_step(4000000 * 2);
@@ -365,6 +396,7 @@ static void check_on_the_fly_freq_change(gconstpointer arg)
     const uint8_t *policy = arg;
     QEMUBH *bh = qemu_bh_new(ptimer_trigger, NULL);
     ptimer_state *ptimer = ptimer_init(bh, *policy);
+    bool no_round_down = (*policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
 
     triggered = false;
 
@@ -374,15 +406,15 @@ static void check_on_the_fly_freq_change(gconstpointer arg)
 
     qemu_clock_step(2000000 * 4 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 4 : 3);
     g_assert_false(triggered);
 
     ptimer_set_freq(ptimer, 250);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 3);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 4 : 3);
 
     qemu_clock_step(2000000 * 4 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 2 : 0);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 4);
@@ -417,13 +449,15 @@ static void check_run_with_delta_0(gconstpointer arg)
     bool no_immediate_trigger = (*policy & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER);
     bool no_immediate_reload = (*policy & PTIMER_POLICY_NO_IMMEDIATE_RELOAD);
     bool continuous_trigger = (*policy & PTIMER_POLICY_CONTINUOUS_TRIGGER);
+    bool no_round_down = (*policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
 
     triggered = false;
 
     ptimer_set_period(ptimer, 2000000);
     ptimer_set_limit(ptimer, 99, 0);
     ptimer_run(ptimer, 1);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_immediate_reload ? 0 : 99);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     no_immediate_reload ? 0 : 99);
 
     if (no_immediate_trigger) {
         g_assert_false(triggered);
@@ -440,12 +474,12 @@ static void check_run_with_delta_0(gconstpointer arg)
 
     qemu_clock_step(2000000 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 97);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 98 : 97);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 97);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 1 : 0);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 2);
@@ -457,7 +491,8 @@ static void check_run_with_delta_0(gconstpointer arg)
 
     ptimer_set_count(ptimer, 0);
     ptimer_run(ptimer, 0);
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_immediate_reload ? 0 : 99);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                     no_immediate_reload ? 0 : 99);
 
     if (no_immediate_trigger) {
         g_assert_false(triggered);
@@ -471,7 +506,8 @@ static void check_run_with_delta_0(gconstpointer arg)
         qemu_clock_step(2000000 + 100000);
 
         if (continuous_trigger || no_immediate_trigger) {
-            g_assert_cmpuint(ptimer_get_count(ptimer), ==, 98);
+            g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+                             no_round_down ? 99 : 98);
             g_assert_true(triggered);
 
             triggered = false;
@@ -484,12 +520,13 @@ static void check_run_with_delta_0(gconstpointer arg)
 
     qemu_clock_step(2000000 + 100000);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, 97);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 98 : 97);
     g_assert_false(triggered);
 
     qemu_clock_step(2000000 * 98);
 
-    g_assert_cmpuint(ptimer_get_count(ptimer), ==, wrap_policy ? 99 : 98);
+    g_assert_cmpuint(ptimer_get_count(ptimer), ==,
+            wrap_policy ? (no_round_down ? 0 : 99) : (no_round_down ? 99 : 98));
     g_assert_true(triggered);
 
     ptimer_stop(ptimer);
@@ -603,6 +640,10 @@ static void add_ptimer_tests(uint8_t policy)
         g_strlcat(policy_name, "no_immediate_reload,", 256);
     }
 
+    if (policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN) {
+        g_strlcat(policy_name, "no_counter_rounddown,", 256);
+    }
+
     qtest_add_data_func(
         g_strdup_printf("/ptimer/set_count policy=%s", policy_name),
         ppolicy, check_set_count);
@@ -650,7 +691,7 @@ static void add_ptimer_tests(uint8_t policy)
 
 static void add_all_ptimer_policies_comb_tests(void)
 {
-    int last_policy = PTIMER_POLICY_NO_IMMEDIATE_RELOAD;
+    int last_policy = PTIMER_POLICY_NO_COUNTER_ROUND_DOWN;
     int policy = PTIMER_POLICY_DEFAULT;
 
     for (; policy < (last_policy << 1); policy++) {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 15/16] arm_mptimer: Convert to use ptimer
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (13 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 14/16] tests: ptimer: Add tests for " Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 16/16] tests: Add tests for the ARM MPTimer Dmitry Osipenko
  2016-09-20 17:25 ` [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Peter Maydell
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

Current ARM MPTimer implementation uses QEMUTimer for the actual timer,
this implementation isn't complete and mostly tries to duplicate of what
generic ptimer is already doing fine.

Conversion to ptimer brings the following benefits and fixes:
	- Simple timer pausing implementation
	- Fixes counter value preservation after stopping the timer
	- Properly handles prescaler != 0 / counter = 0 / load = 0 cases
	- Code simplification and reduction

Bump VMSD to version 3, since VMState is changed and is not compatible
with the previous implementation.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/timer/arm_mptimer.c         | 149 +++++++++++++++++++++++------------------
 include/hw/timer/arm_mptimer.h |   5 +-
 2 files changed, 83 insertions(+), 71 deletions(-)

diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index d66bbf0..daf6c48 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -20,22 +20,33 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/ptimer.h"
 #include "hw/timer/arm_mptimer.h"
 #include "qapi/error.h"
-#include "qemu/timer.h"
+#include "qemu/main-loop.h"
 #include "qom/cpu.h"
 
+#define PTIMER_POLICY                       \
+    (PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD |  \
+     PTIMER_POLICY_CONTINUOUS_TRIGGER    |  \
+     PTIMER_POLICY_NO_IMMEDIATE_TRIGGER  |  \
+     PTIMER_POLICY_NO_IMMEDIATE_RELOAD   |  \
+     PTIMER_POLICY_NO_COUNTER_ROUND_DOWN)
+
 /* This device implements the per-cpu private timer and watchdog block
  * which is used in both the ARM11MPCore and Cortex-A9MP.
  */
 
 static inline int get_current_cpu(ARMMPTimerState *s)
 {
-    if (current_cpu->cpu_index >= s->num_cpu) {
+    int cpu_id = current_cpu ? current_cpu->cpu_index : 0;
+
+    if (cpu_id >= s->num_cpu) {
         hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n",
-                 s->num_cpu, current_cpu->cpu_index);
+                 s->num_cpu, cpu_id);
     }
-    return current_cpu->cpu_index;
+
+    return cpu_id;
 }
 
 static inline void timerblock_update_irq(TimerBlock *tb)
@@ -44,33 +55,42 @@ static inline void timerblock_update_irq(TimerBlock *tb)
 }
 
 /* Return conversion factor from mpcore timer ticks to qemu timer ticks.  */
-static inline uint32_t timerblock_scale(TimerBlock *tb)
+static inline uint32_t timerblock_scale(uint32_t control)
 {
-    return (((tb->control >> 8) & 0xff) + 1) * 10;
+    return (((control >> 8) & 0xff) + 1) * 10;
 }
 
-static void timerblock_reload(TimerBlock *tb, int restart)
+static inline void timerblock_set_count(struct ptimer_state *timer,
+                                        uint32_t control, uint64_t *count)
 {
-    if (tb->count == 0) {
-        return;
+    /* PTimer would trigger interrupt for periodic timer when counter set
+     * to 0, MPtimer under certain condition only.
+     */
+    if ((control & 3) == 3 && (control & 0xff00) == 0 && *count == 0) {
+        *count = ptimer_get_limit(timer);
     }
-    if (restart) {
-        tb->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    ptimer_set_count(timer, *count);
+}
+
+static inline void timerblock_run(struct ptimer_state *timer,
+                                  uint32_t control, uint32_t load)
+{
+    if ((control & 1) && ((control & 0xff00) || load != 0)) {
+        ptimer_run(timer, !(control & 2));
     }
-    tb->tick += (int64_t)tb->count * timerblock_scale(tb);
-    timer_mod(tb->timer, tb->tick);
 }
 
 static void timerblock_tick(void *opaque)
 {
     TimerBlock *tb = (TimerBlock *)opaque;
-    tb->status = 1;
-    if (tb->control & 2) {
-        tb->count = tb->load;
-        timerblock_reload(tb, 0);
-    } else {
-        tb->count = 0;
+    /* Periodic timer with load = 0 and prescaler != 0 would re-trigger
+     * IRQ after one period, otherwise it either stops or wraps around.
+     */
+    if ((tb->control & 2) && (tb->control & 0xff00) == 0 &&
+            ptimer_get_limit(tb->timer) == 0) {
+        ptimer_stop(tb->timer);
     }
+    tb->status = 1;
     timerblock_update_irq(tb);
 }
 
@@ -78,21 +98,11 @@ static uint64_t timerblock_read(void *opaque, hwaddr addr,
                                 unsigned size)
 {
     TimerBlock *tb = (TimerBlock *)opaque;
-    int64_t val;
     switch (addr) {
     case 0: /* Load */
-        return tb->load;
+        return ptimer_get_limit(tb->timer);
     case 4: /* Counter.  */
-        if (((tb->control & 1) == 0) || (tb->count == 0)) {
-            return 0;
-        }
-        /* Slow and ugly, but hopefully won't happen too often.  */
-        val = tb->tick - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        val /= timerblock_scale(tb);
-        if (val < 0) {
-            val = 0;
-        }
-        return val;
+        return ptimer_get_count(tb->timer);
     case 8: /* Control.  */
         return tb->control;
     case 12: /* Interrupt status.  */
@@ -106,37 +116,45 @@ static void timerblock_write(void *opaque, hwaddr addr,
                              uint64_t value, unsigned size)
 {
     TimerBlock *tb = (TimerBlock *)opaque;
-    int64_t old;
+    uint32_t control = tb->control;
     switch (addr) {
     case 0: /* Load */
-        tb->load = value;
-        /* Fall through.  */
-    case 4: /* Counter.  */
-        if ((tb->control & 1) && tb->count) {
-            /* Cancel the previous timer.  */
-            timer_del(tb->timer);
+        /* Setting load to 0 stops the timer without doing the tick if
+         * prescaler = 0.
+         */
+        if ((control & 1) && (control & 0xff00) == 0 && value == 0) {
+            ptimer_stop(tb->timer);
         }
-        tb->count = value;
-        if (tb->control & 1) {
-            timerblock_reload(tb, 1);
+        ptimer_set_limit(tb->timer, value, 1);
+        timerblock_run(tb->timer, control, value);
+        break;
+    case 4: /* Counter.  */
+        /* Setting counter to 0 stops the one-shot timer, or periodic with
+         * load = 0, without doing the tick if prescaler = 0.
+         */
+        if ((control & 1) && (control & 0xff00) == 0 && value == 0 &&
+                (!(control & 2) || ptimer_get_limit(tb->timer) == 0)) {
+            ptimer_stop(tb->timer);
         }
+        timerblock_set_count(tb->timer, control, &value);
+        timerblock_run(tb->timer, control, value);
         break;
     case 8: /* Control.  */
-        old = tb->control;
-        tb->control = value;
+        if ((control & 3) != (value & 3)) {
+            ptimer_stop(tb->timer);
+        }
+        if ((control & 0xff00) != (value & 0xff00)) {
+            ptimer_set_period(tb->timer, timerblock_scale(value));
+        }
         if (value & 1) {
-            if ((old & 1) && (tb->count != 0)) {
-                /* Do nothing if timer is ticking right now.  */
-                break;
+            uint64_t count = ptimer_get_count(tb->timer);
+            /* Re-load periodic timer counter if needed.  */
+            if ((value & 2) && count == 0) {
+                timerblock_set_count(tb->timer, value, &count);
             }
-            if (tb->control & 2) {
-                tb->count = tb->load;
-            }
-            timerblock_reload(tb, 1);
-        } else if (old & 1) {
-            /* Shutdown the timer.  */
-            timer_del(tb->timer);
+            timerblock_run(tb->timer, value, count);
         }
+        tb->control = value;
         break;
     case 12: /* Interrupt status.  */
         tb->status &= ~value;
@@ -186,13 +204,12 @@ static const MemoryRegionOps timerblock_ops = {
 
 static void timerblock_reset(TimerBlock *tb)
 {
-    tb->count = 0;
-    tb->load = 0;
     tb->control = 0;
     tb->status = 0;
-    tb->tick = 0;
     if (tb->timer) {
-        timer_del(tb->timer);
+        ptimer_stop(tb->timer);
+        ptimer_set_limit(tb->timer, 0, 1);
+        ptimer_set_period(tb->timer, timerblock_scale(0));
     }
 }
 
@@ -238,7 +255,8 @@ static void arm_mptimer_realize(DeviceState *dev, Error **errp)
      */
     for (i = 0; i < s->num_cpu; i++) {
         TimerBlock *tb = &s->timerblock[i];
-        tb->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, timerblock_tick, tb);
+        QEMUBH *bh = qemu_bh_new(timerblock_tick, tb);
+        tb->timer = ptimer_init(bh, PTIMER_POLICY);
         sysbus_init_irq(sbd, &tb->irq);
         memory_region_init_io(&tb->iomem, OBJECT(s), &timerblock_ops, tb,
                               "arm_mptimer_timerblock", 0x20);
@@ -248,26 +266,23 @@ static void arm_mptimer_realize(DeviceState *dev, Error **errp)
 
 static const VMStateDescription vmstate_timerblock = {
     .name = "arm_mptimer_timerblock",
-    .version_id = 2,
-    .minimum_version_id = 2,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32(count, TimerBlock),
-        VMSTATE_UINT32(load, TimerBlock),
         VMSTATE_UINT32(control, TimerBlock),
         VMSTATE_UINT32(status, TimerBlock),
-        VMSTATE_INT64(tick, TimerBlock),
-        VMSTATE_TIMER_PTR(timer, TimerBlock),
+        VMSTATE_PTIMER(timer, TimerBlock),
         VMSTATE_END_OF_LIST()
     }
 };
 
 static const VMStateDescription vmstate_arm_mptimer = {
     .name = "arm_mptimer",
-    .version_id = 2,
-    .minimum_version_id = 2,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .fields = (VMStateField[]) {
         VMSTATE_STRUCT_VARRAY_UINT32(timerblock, ARMMPTimerState, num_cpu,
-                                     2, vmstate_timerblock, TimerBlock),
+                                     3, vmstate_timerblock, TimerBlock),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/include/hw/timer/arm_mptimer.h b/include/hw/timer/arm_mptimer.h
index b34cba0..c46d8d2 100644
--- a/include/hw/timer/arm_mptimer.h
+++ b/include/hw/timer/arm_mptimer.h
@@ -27,12 +27,9 @@
 
 /* State of a single timer or watchdog block */
 typedef struct {
-    uint32_t count;
-    uint32_t load;
     uint32_t control;
     uint32_t status;
-    int64_t tick;
-    QEMUTimer *timer;
+    struct ptimer_state *timer;
     qemu_irq irq;
     MemoryRegion iomem;
 } TimerBlock;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v16 16/16] tests: Add tests for the ARM MPTimer
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (14 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 15/16] arm_mptimer: Convert to use ptimer Dmitry Osipenko
@ 2016-09-07 13:22 ` Dmitry Osipenko
  2016-09-20 17:25 ` [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Peter Maydell
  16 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 13:22 UTC (permalink / raw)
  To: QEMU Developers, qemu-arm; +Cc: Peter Crosthwaite, Peter Maydell

ARM MPTimer is a per-CPU core timer, essential part of the ARM Cortex-A9
MPCore. Add QTests for it.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 tests/Makefile.include   |    3 +
 tests/test-arm-mptimer.c | 1105 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 1108 insertions(+)
 create mode 100644 tests/test-arm-mptimer.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 82b7ae5..4cd3d08 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -258,6 +258,8 @@ check-qtest-arm-y += tests/ds1338-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
 check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
 gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
+check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
+gcov-files-arm-y += hw/timer/arm_mptimer.c
 check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc-y += tests/drive_del-test$(EXESUF)
@@ -633,6 +635,7 @@ tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-ob
 tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
 tests/ptimer-test$(EXESUF): tests/ptimer-test.o tests/ptimer-test-stubs.o hw/core/ptimer.o
+tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
 	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"  LINK  $(TARGET_DIR)$@")
diff --git a/tests/test-arm-mptimer.c b/tests/test-arm-mptimer.c
new file mode 100644
index 0000000..f73185b
--- /dev/null
+++ b/tests/test-arm-mptimer.c
@@ -0,0 +1,1105 @@
+/*
+ * QTest testcase for the ARM MPTimer
+ *
+ * Author: Dmitry Osipenko <digetx@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/timer.h"
+#include "libqtest.h"
+
+#define TIMER_BLOCK_SCALE(s)    ((((s) & 0xff) + 1) * 10)
+
+#define TIMER_BLOCK_STEP(scaler, steps_nb) \
+    clock_step(TIMER_BLOCK_SCALE(scaler) * (int64_t)(steps_nb) + 1)
+
+#define TIMER_BASE_PHYS 0x1e000600
+
+#define TIMER_LOAD      0x00
+#define TIMER_COUNTER   0x04
+#define TIMER_CONTROL   0x08
+#define TIMER_INTSTAT   0x0C
+
+#define TIMER_CONTROL_ENABLE        (1 << 0)
+#define TIMER_CONTROL_PERIODIC      (1 << 1)
+#define TIMER_CONTROL_IT_ENABLE     (1 << 2)
+#define TIMER_CONTROL_PRESCALER(p)  (((p) & 0xff) << 8)
+
+#define PERIODIC     1
+#define ONESHOT      0
+#define NOSCALE      0
+
+static int nonscaled = NOSCALE;
+static int scaled = 122;
+
+static void timer_load(uint32_t load)
+{
+    writel(TIMER_BASE_PHYS + TIMER_LOAD, load);
+}
+
+static void timer_start(int periodic, uint32_t scale)
+{
+    uint32_t ctl = TIMER_CONTROL_ENABLE | TIMER_CONTROL_PRESCALER(scale);
+
+    if (periodic) {
+        ctl |= TIMER_CONTROL_PERIODIC;
+    }
+
+    writel(TIMER_BASE_PHYS + TIMER_CONTROL, ctl);
+}
+
+static void timer_stop(void)
+{
+    writel(TIMER_BASE_PHYS + TIMER_CONTROL, 0);
+}
+
+static void timer_int_clr(void)
+{
+    writel(TIMER_BASE_PHYS + TIMER_INTSTAT, 1);
+}
+
+static void timer_reset(void)
+{
+    timer_stop();
+    timer_load(0);
+    timer_int_clr();
+}
+
+static uint32_t timer_get_and_clr_int_sts(void)
+{
+    uint32_t int_sts = readl(TIMER_BASE_PHYS + TIMER_INTSTAT);
+
+    if (int_sts) {
+        timer_int_clr();
+    }
+
+    return int_sts;
+}
+
+static uint32_t timer_counter(void)
+{
+    return readl(TIMER_BASE_PHYS + TIMER_COUNTER);
+}
+
+static void timer_set_counter(uint32_t value)
+{
+    writel(TIMER_BASE_PHYS + TIMER_COUNTER, value);
+}
+
+static void test_timer_oneshot(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(9999999);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 9999);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+    g_assert_cmpuint(timer_counter(), ==, 9990000);
+
+    TIMER_BLOCK_STEP(scaler, 9990000);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+    TIMER_BLOCK_STEP(scaler, 9990000);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_pause(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(999999999);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 999);
+
+    g_assert_cmpuint(timer_counter(), ==, 999999000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(scaler, 9000);
+
+    g_assert_cmpuint(timer_counter(), ==, 999990000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_stop();
+
+    g_assert_cmpuint(timer_counter(), ==, 999990000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(scaler, 90000);
+
+    g_assert_cmpuint(timer_counter(), ==, 999990000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 999990000);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    TIMER_BLOCK_STEP(scaler, 999990000);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+}
+
+static void test_timer_reload(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 90000);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 90000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_load(UINT32_MAX);
+
+    TIMER_BLOCK_STEP(scaler, 90000);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 90000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_periodic(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+    int repeat = 10;
+
+    timer_reset();
+    timer_load(100);
+    timer_start(PERIODIC, scaler);
+
+    while (repeat--) {
+        clock_step(TIMER_BLOCK_SCALE(scaler) * (101 + repeat) + 1);
+
+        g_assert_cmpuint(timer_counter(), ==, 100 - repeat);
+        g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+        clock_step(TIMER_BLOCK_SCALE(scaler) * (101 - repeat) - 1);
+    }
+}
+
+static void test_timer_oneshot_to_periodic(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(10000);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1000);
+
+    g_assert_cmpuint(timer_counter(), ==, 9000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 14001);
+
+    g_assert_cmpuint(timer_counter(), ==, 5000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+}
+
+static void test_timer_periodic_to_oneshot(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(99999999);
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 999);
+
+    g_assert_cmpuint(timer_counter(), ==, 99999000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 99999009);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+}
+
+static void test_timer_prescaler(void)
+{
+    timer_reset();
+    timer_load(9999999);
+    timer_start(ONESHOT, NOSCALE);
+
+    TIMER_BLOCK_STEP(NOSCALE, 9999998);
+
+    g_assert_cmpuint(timer_counter(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(NOSCALE, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+    timer_reset();
+    timer_load(9999999);
+    timer_start(ONESHOT, 0xAB);
+
+    TIMER_BLOCK_STEP(0xAB, 9999998);
+
+    g_assert_cmpuint(timer_counter(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(0xAB, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+}
+
+static void test_timer_prescaler_on_the_fly(void)
+{
+    timer_reset();
+    timer_load(9999999);
+    timer_start(ONESHOT, NOSCALE);
+
+    TIMER_BLOCK_STEP(NOSCALE, 999);
+
+    g_assert_cmpuint(timer_counter(), ==, 9999000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(ONESHOT, 0xAB);
+
+    TIMER_BLOCK_STEP(0xAB, 9000);
+
+    g_assert_cmpuint(timer_counter(), ==, 9990000);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_set_oneshot_counter_to_0(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_set_counter(0);
+
+    TIMER_BLOCK_STEP(scaler, 10);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_set_periodic_counter_to_0(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_set_counter(0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - (scaler ? 0 : 1));
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    timer_reset();
+    timer_set_counter(UINT32_MAX);
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_set_counter(0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_noload_oneshot(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_noload_periodic(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_zero_load_oneshot(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_zero_load_periodic(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_zero_load_oneshot_to_nonzero(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    timer_load(999);
+
+    TIMER_BLOCK_STEP(scaler, 1001);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+}
+
+static void test_timer_zero_load_periodic_to_nonzero(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+    int i;
+
+    timer_reset();
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    timer_load(1999999);
+
+    for (i = 1; i < 10; i++) {
+        TIMER_BLOCK_STEP(scaler, 2000001);
+
+        g_assert_cmpuint(timer_counter(), ==, 1999999 - i);
+        g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+        g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+    }
+}
+
+static void test_timer_nonzero_load_oneshot_to_zero(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    timer_load(UINT32_MAX);
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_nonzero_load_periodic_to_zero(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    timer_load(UINT32_MAX);
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_set_periodic_counter_on_the_fly(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(UINT32_MAX / 2);
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX / 2 - 100);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_set_counter(UINT32_MAX);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 100);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_enable_and_set_counter(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    timer_set_counter(UINT32_MAX);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 100);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_set_counter_and_enable(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_set_counter(UINT32_MAX);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 100);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_set_counter_disabled(void)
+{
+    timer_reset();
+    timer_set_counter(999999999);
+
+    TIMER_BLOCK_STEP(NOSCALE, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 999999999);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_load_disabled(void)
+{
+    timer_reset();
+    timer_load(999999999);
+
+    TIMER_BLOCK_STEP(NOSCALE, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 999999999);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_oneshot_with_counter_0_on_start(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(999);
+    timer_set_counter(0);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_periodic_with_counter_0_on_start(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+    int i;
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_set_counter(0);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX + (scaler ? 1 : 0) - 100);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX + (scaler ? 1 : 0) - 200);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_reset();
+    timer_load(1999999);
+    timer_set_counter(0);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    for (i = 2 - (!!scaler ? 1 : 0); i < 10; i++) {
+        TIMER_BLOCK_STEP(scaler, 2000001);
+
+        g_assert_cmpuint(timer_counter(), ==, 1999999 - i);
+        g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+        g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+    }
+}
+
+static void test_periodic_counter(gconstpointer arg)
+{
+    const int test_load = 10;
+    int scaler = *((int *) arg);
+    int test_val;
+
+    timer_reset();
+    timer_load(test_load);
+    timer_start(PERIODIC, scaler);
+
+    clock_step(1);
+
+    for (test_val = 0; test_val <= test_load; test_val++) {
+        clock_step(TIMER_BLOCK_SCALE(scaler) * test_load);
+        g_assert_cmpint(timer_counter(), ==, test_val);
+    }
+}
+
+static void test_timer_set_counter_periodic_with_zero_load(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_start(PERIODIC, scaler);
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    timer_set_counter(999);
+
+    TIMER_BLOCK_STEP(scaler, 999);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_set_oneshot_load_to_0(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 100);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_set_periodic_load_to_0(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, UINT32_MAX - 100);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_load(0);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 100);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+    g_assert_cmpuint(timer_counter(), ==, 0);
+}
+
+static void test_deferred_trigger(void)
+{
+    int mode = ONESHOT;
+
+again:
+    timer_reset();
+    timer_start(mode, 255);
+
+    clock_step(100);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+    timer_reset();
+    timer_load(2);
+    timer_start(mode, 255);
+
+    clock_step(100);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_start(mode, 255);
+
+    clock_step(100);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_set_counter(0);
+
+    clock_step(100);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+    timer_reset();
+    timer_load(UINT32_MAX);
+    timer_start(mode, 255);
+
+    clock_step(100);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_load(0);
+
+    clock_step(100);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+
+    if (mode == ONESHOT) {
+        mode = PERIODIC;
+        goto again;
+    }
+}
+
+static void test_timer_zero_load_mode_switch(gconstpointer arg)
+{
+    int scaler = *((int *) arg);
+
+    timer_reset();
+    timer_load(0);
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    timer_start(ONESHOT, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    timer_start(PERIODIC, scaler);
+
+    TIMER_BLOCK_STEP(scaler, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, !!scaler);
+}
+
+static void test_timer_zero_load_prescaled_periodic_to_nonscaled_oneshot(void)
+{
+    timer_reset();
+    timer_load(0);
+    timer_start(PERIODIC, 255);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    timer_start(ONESHOT, NOSCALE);
+
+    TIMER_BLOCK_STEP(NOSCALE, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(NOSCALE, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_zero_load_prescaled_oneshot_to_nonscaled_periodic(void)
+{
+    timer_reset();
+    timer_load(0);
+    timer_start(ONESHOT, 255);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(PERIODIC, NOSCALE);
+
+    TIMER_BLOCK_STEP(NOSCALE, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_zero_load_nonscaled_oneshot_to_prescaled_periodic(void)
+{
+    timer_reset();
+    timer_load(0);
+    timer_start(ONESHOT, NOSCALE);
+
+    TIMER_BLOCK_STEP(NOSCALE, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(PERIODIC, 255);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+static void test_timer_zero_load_nonscaled_periodic_to_prescaled_oneshot(void)
+{
+    timer_reset();
+    timer_load(0);
+    timer_start(PERIODIC, NOSCALE);
+
+    TIMER_BLOCK_STEP(NOSCALE, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    timer_start(ONESHOT, 255);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 1);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+
+    TIMER_BLOCK_STEP(255, 1);
+
+    g_assert_cmpuint(timer_counter(), ==, 0);
+    g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
+}
+
+int main(int argc, char **argv)
+{
+    int *scaler = &nonscaled;
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("mptimer/deferred_trigger", test_deferred_trigger);
+    qtest_add_func("mptimer/load_disabled", test_timer_load_disabled);
+    qtest_add_func("mptimer/set_counter_disabled", test_timer_set_counter_disabled);
+    qtest_add_func("mptimer/zero_load_prescaled_periodic_to_nonscaled_oneshot",
+                   test_timer_zero_load_prescaled_periodic_to_nonscaled_oneshot);
+    qtest_add_func("mptimer/zero_load_prescaled_oneshot_to_nonscaled_periodic",
+                   test_timer_zero_load_prescaled_oneshot_to_nonscaled_periodic);
+    qtest_add_func("mptimer/zero_load_nonscaled_oneshot_to_prescaled_periodic",
+                   test_timer_zero_load_nonscaled_oneshot_to_prescaled_periodic);
+    qtest_add_func("mptimer/zero_load_nonscaled_periodic_to_prescaled_oneshot",
+                   test_timer_zero_load_nonscaled_periodic_to_prescaled_oneshot);
+    qtest_add_func("mptimer/prescaler", test_timer_prescaler);
+    qtest_add_func("mptimer/prescaler_on_the_fly", test_timer_prescaler_on_the_fly);
+
+tests_with_prescaler_arg:
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/oneshot scaler=%d", *scaler),
+                        scaler, test_timer_oneshot);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/pause scaler=%d", *scaler),
+                        scaler, test_timer_pause);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/reload scaler=%d", *scaler),
+                        scaler, test_timer_reload);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/periodic scaler=%d", *scaler),
+                        scaler, test_timer_periodic);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/oneshot_to_periodic scaler=%d", *scaler),
+                        scaler, test_timer_oneshot_to_periodic);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/periodic_to_oneshot scaler=%d", *scaler),
+                        scaler, test_timer_periodic_to_oneshot);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/set_oneshot_counter_to_0 scaler=%d", *scaler),
+                        scaler, test_timer_set_oneshot_counter_to_0);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/set_periodic_counter_to_0 scaler=%d", *scaler),
+                        scaler, test_timer_set_periodic_counter_to_0);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/noload_oneshot scaler=%d", *scaler),
+                        scaler, test_timer_noload_oneshot);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/noload_periodic scaler=%d", *scaler),
+                        scaler, test_timer_noload_periodic);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/zero_load_oneshot scaler=%d", *scaler),
+                        scaler, test_timer_zero_load_oneshot);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/zero_load_periodic scaler=%d", *scaler),
+                        scaler, test_timer_zero_load_periodic);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/zero_load_oneshot_to_nonzero scaler=%d", *scaler),
+                        scaler, test_timer_zero_load_oneshot_to_nonzero);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/zero_load_periodic_to_nonzero scaler=%d", *scaler),
+                        scaler, test_timer_zero_load_periodic_to_nonzero);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/nonzero_load_oneshot_to_zero scaler=%d", *scaler),
+                        scaler, test_timer_nonzero_load_oneshot_to_zero);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/nonzero_load_periodic_to_zero scaler=%d", *scaler),
+                        scaler, test_timer_nonzero_load_periodic_to_zero);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/set_periodic_counter_on_the_fly scaler=%d", *scaler),
+                        scaler, test_timer_set_periodic_counter_on_the_fly);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/enable_and_set_counter scaler=%d", *scaler),
+                        scaler, test_timer_enable_and_set_counter);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/set_counter_and_enable scaler=%d", *scaler),
+                        scaler, test_timer_set_counter_and_enable);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/oneshot_with_counter_0_on_start scaler=%d", *scaler),
+                        scaler, test_timer_oneshot_with_counter_0_on_start);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/periodic_with_counter_0_on_start scaler=%d", *scaler),
+                        scaler, test_timer_periodic_with_counter_0_on_start);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/periodic_counter scaler=%d", *scaler),
+                        scaler, test_periodic_counter);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/set_counter_periodic_with_zero_load scaler=%d", *scaler),
+                        scaler, test_timer_set_counter_periodic_with_zero_load);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/set_oneshot_load_to_0 scaler=%d", *scaler),
+                        scaler, test_timer_set_oneshot_load_to_0);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/set_periodic_load_to_0 scaler=%d", *scaler),
+                        scaler, test_timer_set_periodic_load_to_0);
+    qtest_add_data_func(
+        g_strdup_printf("mptimer/zero_load_mode_switch scaler=%d", *scaler),
+                        scaler, test_timer_zero_load_mode_switch);
+
+    if (scaler == &nonscaled) {
+        scaler = &scaled;
+        goto tests_with_prescaler_arg;
+    }
+
+    qtest_start("-machine vexpress-a9");
+    ret = g_test_run();
+    qtest_end();
+
+    return ret;
+}
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests Dmitry Osipenko
@ 2016-09-07 21:32   ` Eric Blake
  2016-09-07 22:32     ` Dmitry Osipenko
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Blake @ 2016-09-07 21:32 UTC (permalink / raw)
  To: Dmitry Osipenko, QEMU Developers, qemu-arm
  Cc: Peter Maydell, Peter Crosthwaite

[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]

On 09/07/2016 08:22 AM, Dmitry Osipenko wrote:
> Ptimer is a generic countdown timer helper that is used by many timer
> device models as well as by the QEMU core. Add QTests for the ptimer.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---

> --- /dev/null
> +++ b/tests/ptimer-test-stubs.c
> @@ -0,0 +1,107 @@
> +/*
> + * Stubs for the ptimer-test
> + *
> + * Author: Dmitry Osipenko <digetx@gmail.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *

I'm not a lawyer, but my reading of the GPL says that for it to be
enforceable, someone has to claim copyright (in other words, it works
BECAUSE of copyright law).  Yeah, you may already have implicit rights
based on country of origin, but it's always safer to add an explicit
Copyright line; if for no other reason than to be consistent with what
most other files do.

Also, are these new files covered by an addition to MAINTAINERS?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests
  2016-09-07 21:32   ` Eric Blake
@ 2016-09-07 22:32     ` Dmitry Osipenko
  2016-09-07 23:04       ` Peter Maydell
  0 siblings, 1 reply; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 22:32 UTC (permalink / raw)
  To: Eric Blake, QEMU Developers, qemu-arm; +Cc: Peter Maydell, Peter Crosthwaite

On 08.09.2016 00:32, Eric Blake wrote:
> On 09/07/2016 08:22 AM, Dmitry Osipenko wrote:
>> Ptimer is a generic countdown timer helper that is used by many timer
>> device models as well as by the QEMU core. Add QTests for the ptimer.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
> 
>> --- /dev/null
>> +++ b/tests/ptimer-test-stubs.c
>> @@ -0,0 +1,107 @@
>> +/*
>> + * Stubs for the ptimer-test
>> + *
>> + * Author: Dmitry Osipenko <digetx@gmail.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
> 
> I'm not a lawyer, but my reading of the GPL says that for it to be
> enforceable, someone has to claim copyright (in other words, it works
> BECAUSE of copyright law).  Yeah, you may already have implicit rights
> based on country of origin, but it's always safer to add an explicit
> Copyright line; if for no other reason than to be consistent with what
> most other files do.
> 

Thank you for the suggestion, I'll s/Author:/Copyright (C) 2016,/. The
consistency with other files is arguable, because that variant I choose by
looking in several files and picking the most popular.

> Also, are these new files covered by an addition to MAINTAINERS?
> 

No, they are not. Not every file in tests/ covered by MAINTAINERS,
get_maintainer script returns sane suggestion for them based on the recent
contributors.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests
  2016-09-07 22:32     ` Dmitry Osipenko
@ 2016-09-07 23:04       ` Peter Maydell
  2016-09-07 23:25         ` Dmitry Osipenko
  0 siblings, 1 reply; 26+ messages in thread
From: Peter Maydell @ 2016-09-07 23:04 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: Eric Blake, QEMU Developers, qemu-arm, Peter Crosthwaite

On 7 September 2016 at 23:32, Dmitry Osipenko <digetx@gmail.com> wrote:
> Thank you for the suggestion, I'll s/Author:/Copyright (C) 2016,/. The
> consistency with other files is arguable, because that variant I choose by
> looking in several files and picking the most popular.

Most files with an Author: line have it because the file is
Copyright Some Company, and then the Author line comes after
it and gives credit to the individual working for the company
who wrote the code.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests
  2016-09-07 23:04       ` Peter Maydell
@ 2016-09-07 23:25         ` Dmitry Osipenko
  0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-07 23:25 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Eric Blake, QEMU Developers, qemu-arm, Peter Crosthwaite

On 08.09.2016 02:04, Peter Maydell wrote:
> On 7 September 2016 at 23:32, Dmitry Osipenko <digetx@gmail.com> wrote:
>> Thank you for the suggestion, I'll s/Author:/Copyright (C) 2016,/. The
>> consistency with other files is arguable, because that variant I choose by
>> looking in several files and picking the most popular.
> 
> Most files with an Author: line have it because the file is
> Copyright Some Company, and then the Author line comes after
> it and gives credit to the individual working for the company
> who wrote the code.
> 

Thank you for the clarification.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 05/16] hw/ptimer: Add "wraparound after one period" policy
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 05/16] hw/ptimer: Add "wraparound after one period" policy Dmitry Osipenko
@ 2016-09-20 17:20   ` Peter Maydell
  2016-09-20 20:57     ` Dmitry Osipenko
  0 siblings, 1 reply; 26+ messages in thread
From: Peter Maydell @ 2016-09-20 17:20 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite

On 7 September 2016 at 14:22, Dmitry Osipenko <digetx@gmail.com> wrote:
> Currently, periodic counter wraps around immediately once counter reaches
> "0", this is wrong behaviour for some of the timers, resulting in one period
> being lost. Add new ptimer policy that provides correct behaviour for such
> timers, so that counter stays with "0" for a one period before wrapping
> around.

This says it's just adding a new policy...

> @@ -91,7 +96,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
>  {
>      uint64_t counter;
>
> -    if (s->enabled) {
> +    if (s->enabled && s->delta != 0) {
>          int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>          int64_t next = s->next_event;
>          bool expired = (now - next >= 0);
> @@ -145,6 +150,14 @@ uint64_t ptimer_get_count(ptimer_state *s)
>                      div += 1;
>              }
>              counter = rem / div;
> +
> +            if (!oneshot && s->delta == s->limit) {
> +                /* Before wrapping around, timer should stay with counter = 0
> +                   for a one period.  The delta has been adjusted by +1 for
> +                   the wrapped around counter, so taking a modulo of limit + 1
> +                   gives that period.  */
> +                counter %= s->limit + 1;
> +            }
>          }
>      } else {
>          counter = s->delta;

...but the changes in this function look like they affect
behaviour even if that policy flag isn't set.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy Dmitry Osipenko
@ 2016-09-20 17:21   ` Peter Maydell
  2016-09-20 21:06     ` Dmitry Osipenko
  0 siblings, 1 reply; 26+ messages in thread
From: Peter Maydell @ 2016-09-20 17:21 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite

On 7 September 2016 at 14:22, Dmitry Osipenko <digetx@gmail.com> wrote:
> Currently, periodic timer that has load = delta = 0 performs trigger
> on timer reload and stops, printing a "period zero" error message.
> Introduce new policy that makes periodic timer to continuously trigger
> with a period interval in case of load = 0.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  hw/core/ptimer.c    | 18 +++++++++++++++++-
>  include/hw/ptimer.h |  4 ++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
> index e9b2e15..97ce8ae 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -45,7 +45,8 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
>          ptimer_trigger(s);
>          delta = s->delta = s->limit;
>      }
> -    if (delta == 0 || s->period == 0) {
> +
> +    if (s->period == 0) {
>          if (!qtest_enabled()) {
>              fprintf(stderr, "Timer with period zero, disabling\n");
>          }
> @@ -58,6 +59,21 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
>          delta += delta_adjust;
>      }
>
> +    if (delta == 0 && (s->policy_mask & PTIMER_POLICY_CONTINUOUS_TRIGGER)) {
> +        if (s->enabled == 1) {
> +            delta = 1;
> +        }
> +    }
> +
> +    if (delta == 0) {
> +        if (!qtest_enabled()) {
> +            fprintf(stderr, "Timer with delta zero, disabling\n");
> +        }
> +        timer_del(s->timer);
> +        s->enabled = 0;
> +        return;
> +    }
> +

Again, this looks like it's affecting behaviour even when the
policy flag isn't set.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion
  2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
                   ` (15 preceding siblings ...)
  2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 16/16] tests: Add tests for the ARM MPTimer Dmitry Osipenko
@ 2016-09-20 17:25 ` Peter Maydell
  16 siblings, 0 replies; 26+ messages in thread
From: Peter Maydell @ 2016-09-20 17:25 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite

On 7 September 2016 at 14:22, Dmitry Osipenko <digetx@gmail.com> wrote:
> Hello,
>
> Currently, QEMU ARM MPTimer device model provides only a certain subset of
> the emulation behavior. This patch series is supposed to add missing parts by
> converting the MPTimer to use generic ptimer helper. It fixes some important
> ptimer bugs and provides new features that are required for the ARM MPTimer.
>
> Dmitry Osipenko (16):
>   hw/ptimer: Actually stop the timer in case of error
>   hw/ptimer: Introduce timer policy feature
>   tests: Add ptimer tests
>   hw/ptimer: Suppress error messages under qtest

I've applied these first four patches to target-arm.next
(there were review issues with 5 and onward).

thanks
-- PMM

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 05/16] hw/ptimer: Add "wraparound after one period" policy
  2016-09-20 17:20   ` Peter Maydell
@ 2016-09-20 20:57     ` Dmitry Osipenko
  0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-20 20:57 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite

On 20.09.2016 20:20, Peter Maydell wrote:
> On 7 September 2016 at 14:22, Dmitry Osipenko <digetx@gmail.com> wrote:
>> Currently, periodic counter wraps around immediately once counter reaches
>> "0", this is wrong behaviour for some of the timers, resulting in one period
>> being lost. Add new ptimer policy that provides correct behaviour for such
>> timers, so that counter stays with "0" for a one period before wrapping
>> around.
> 
> This says it's just adding a new policy...
> 
>> @@ -91,7 +96,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
>>  {
>>      uint64_t counter;
>>
>> -    if (s->enabled) {
>> +    if (s->enabled && s->delta != 0) {
>>          int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>>          int64_t next = s->next_event;
>>          bool expired = (now - next >= 0);
>> @@ -145,6 +150,14 @@ uint64_t ptimer_get_count(ptimer_state *s)
>>                      div += 1;
>>              }
>>              counter = rem / div;
>> +
>> +            if (!oneshot && s->delta == s->limit) {
>> +                /* Before wrapping around, timer should stay with counter = 0
>> +                   for a one period.  The delta has been adjusted by +1 for
>> +                   the wrapped around counter, so taking a modulo of limit + 1
>> +                   gives that period.  */
>> +                counter %= s->limit + 1;
>> +            }
>>          }
>>      } else {
>>          counter = s->delta;
> 
> ...but the changes in this function look like they affect
> behaviour even if that policy flag isn't set.
> 

No, the delta value is adjusted only when PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD is
set.

+    if (s->policy_mask & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD) {
+        delta += delta_adjust;
+    }
+

That adjustment is applied only on reload of the periodic timer.

@@ -83,7 +88,7 @@ static void ptimer_tick(void *opaque)
     if (s->enabled == 2) {
         s->enabled = 0;
     } else {
-        ptimer_reload(s);
+        ptimer_reload(s, 1);
     }
 }

All other ptimer_reload's are ptimer_reload(s, 0).

The periodic timer is reloaded with the limit value. When s->delta == s->limit,
we can assume that timer is free running. When delta is adjusted by +1 on
reload, the counter = limit + 1 (counter value is calculated based on elapsed
QEMU time) and that "counter %= s->limit + 1" modulo gives counter = 0 while in
fact the counter = adjusted delta (limit + 1).

When delta is *not* adjusted, the modulo returns the actual counter value, since
counter value is always less than s->limit + 1.

So, this patch doesn't affect old behaviour at all.

Looking more at it now, I think "counter %= s->limit + 1" should be changed to
"counter %= s->limit" in this patch and +1 added in the "no counter round down"
patch, since the counter value is always rounded down here. Instead of the
"staying with counter = 0 for a one period" it would wraparound to the limit
value if "wraparound after one period" policy is used. I'll change it in V17.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy
  2016-09-20 17:21   ` Peter Maydell
@ 2016-09-20 21:06     ` Dmitry Osipenko
  0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2016-09-20 21:06 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite

On 20.09.2016 20:21, Peter Maydell wrote:
> On 7 September 2016 at 14:22, Dmitry Osipenko <digetx@gmail.com> wrote:
>> Currently, periodic timer that has load = delta = 0 performs trigger
>> on timer reload and stops, printing a "period zero" error message.
>> Introduce new policy that makes periodic timer to continuously trigger
>> with a period interval in case of load = 0.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  hw/core/ptimer.c    | 18 +++++++++++++++++-
>>  include/hw/ptimer.h |  4 ++++
>>  2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
>> index e9b2e15..97ce8ae 100644
>> --- a/hw/core/ptimer.c
>> +++ b/hw/core/ptimer.c
>> @@ -45,7 +45,8 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
>>          ptimer_trigger(s);
>>          delta = s->delta = s->limit;
>>      }
>> -    if (delta == 0 || s->period == 0) {
>> +
>> +    if (s->period == 0) {
>>          if (!qtest_enabled()) {
>>              fprintf(stderr, "Timer with period zero, disabling\n");
>>          }
>> @@ -58,6 +59,21 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
>>          delta += delta_adjust;
>>      }
>>
>> +    if (delta == 0 && (s->policy_mask & PTIMER_POLICY_CONTINUOUS_TRIGGER)) {
>> +        if (s->enabled == 1) {
>> +            delta = 1;
>> +        }
>> +    }
>> +
>> +    if (delta == 0) {
>> +        if (!qtest_enabled()) {
>> +            fprintf(stderr, "Timer with delta zero, disabling\n");
>> +        }
>> +        timer_del(s->timer);
>> +        s->enabled = 0;
>> +        return;
>> +    }
>> +
> 
> Again, this looks like it's affecting behaviour even when the
> policy flag isn't set.
> 

No, the old behaviour is "to error out" when the delta = 0 and it's still the
same with this patch applied. This patch splits the "(delta == 0 || s->period ==
0)" check, so it won't error out if delta was changed to 1 in case of the
"continuous trigger" policy being used.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2016-09-20 21:07 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-07 13:22 [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 01/16] hw/ptimer: Actually stop the timer in case of error Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 02/16] hw/ptimer: Introduce timer policy feature Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 03/16] tests: Add ptimer tests Dmitry Osipenko
2016-09-07 21:32   ` Eric Blake
2016-09-07 22:32     ` Dmitry Osipenko
2016-09-07 23:04       ` Peter Maydell
2016-09-07 23:25         ` Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 04/16] hw/ptimer: Suppress error messages under qtest Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 05/16] hw/ptimer: Add "wraparound after one period" policy Dmitry Osipenko
2016-09-20 17:20   ` Peter Maydell
2016-09-20 20:57     ` Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 06/16] tests: ptimer: Add tests for " Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 07/16] hw/ptimer: Add "continuous trigger" policy Dmitry Osipenko
2016-09-20 17:21   ` Peter Maydell
2016-09-20 21:06     ` Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 08/16] tests: ptimer: Add tests for " Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 09/16] hw/ptimer: Add "no immediate " Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 10/16] tests: ptimer: Add tests for " Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 11/16] hw/ptimer: Add "no immediate reload" policy Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 12/16] tests: ptimer: Add tests for " Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 13/16] hw/ptimer: Add "no counter round down" policy Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 14/16] tests: ptimer: Add tests for " Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 15/16] arm_mptimer: Convert to use ptimer Dmitry Osipenko
2016-09-07 13:22 ` [Qemu-devel] [PATCH v16 16/16] tests: Add tests for the ARM MPTimer Dmitry Osipenko
2016-09-20 17:25 ` [Qemu-devel] [PATCH v16 00/16] PTimer fixes/features and ARM MPTimer conversion Peter Maydell

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.