All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64()
@ 2015-08-27 19:32 Laurent Vivier
  2015-08-27 19:32 ` [Qemu-devel] [PATCH v3 1/9] i6300esb: remove muldiv64() Laurent Vivier
                   ` (9 more replies)
  0 siblings, 10 replies; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:32 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell, Stefan Hajnoczi, Michael S . Tsirkin,
	Paolo Bonzini, Aurelien Jarno, Leon Alrae, Jia Liu, Jason Wang,
	crosthwaitepeter, qemu-trivial

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds, by
doing something like:

     y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)

where x is the number of device ticks and y the number of system ticks.

y is used as nanoseconds in timer functions,
it works because 1 tick is 1 nanosecond.
(get_ticks_per_sec() is 10^9)

But if get_ticks_per_sec() / TIMER_FREQ is an integer, we can do:

    y = x * TIMER_PERIOD;

v3:
Split "PCI: remove muldiv64()" in 3 patches:
    i6300esb: remove muldiv64()
    rtl8139: remove muldiv64()
    pcnet: remove muldiv64()
v2:
4/4 For target-arm, don't remove muldiv64() but clarify
    the use of the values.
7/7 Replace qemu_clock_get_ns()/1000 by qemu_clock_get_us()

Laurent Vivier (9):
  i6300esb: remove muldiv64()
  rtl8139: remove muldiv64()
  pcnet: remove muldiv64()
  mips: remove muldiv64()
  openrisc: remove muldiv64()
  arm: clarify the use of muldiv64()
  hpet: remove muldiv64()
  bt: remove muldiv64()
  net: remove muldiv64()

 hw/bt/hci.c                |  4 ++--
 hw/mips/cputimer.c         | 19 ++++++++-----------
 hw/net/pcnet.c             |  3 +--
 hw/net/rtl8139.c           | 14 ++++++--------
 hw/openrisc/cputimer.c     |  7 +++----
 hw/timer/hpet.c            |  6 +++---
 hw/watchdog/wdt_i6300esb.c | 11 +++--------
 include/hw/timer/hpet.h    |  4 ++--
 net/dump.c                 |  2 +-
 target-arm/helper.c        | 14 ++++++++------
 tests/rtl8139-test.c       |  2 +-
 11 files changed, 38 insertions(+), 48 deletions(-)

-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 1/9] i6300esb: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
@ 2015-08-27 19:32 ` Laurent Vivier
  2015-09-14 13:31   ` [Qemu-devel] [PATCH v4 " Laurent Vivier
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 2/9] rtl8139: " Laurent Vivier
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:32 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Michael S . Tsirkin, Paolo Bonzini,
	qemu-trivial

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds, by
doing something like:

    y = muldiv64(x, get_ticks_per_sec(), PCI_FREQUENCY)

where x is the number of device ticks and y the number of system ticks.

y is used as nanoseconds in timer functions,
it works because 1 tick is 1 nanosecond.
(get_ticks_per_sec() is 10^9)

But as PCI frequency is 33 MHz, we can also do:

    y = x * 30; /* 33 MHz PCI period is 30 ns */

Which is much more simple.

This implies a 33.333333 MHz PCI frequency,
but this is correct.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v3: part of "PCI: remove muldiv64()"
 hw/watchdog/wdt_i6300esb.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index cfa2b1b..a91c8fd 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -129,14 +129,9 @@ static void i6300esb_restart_timer(I6300State *d, int stage)
     else
         timeout <<= 5;
 
-    /* Get the timeout in units of ticks_per_sec.
-     *
-     * ticks_per_sec is typically 10^9 == 0x3B9ACA00 (30 bits), with
-     * 20 bits of user supplied preload, and 15 bits of scale, the
-     * multiply here can exceed 64-bits, before we divide by 33MHz, so
-     * we use a higher-precision intermediate result.
-     */
-    timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000);
+    /* Get the timeout in nanoseconds. */
+
+    timeout = timeout * 30; /* on a PCI bus, 1 tick is 30 ns*/
 
     i6300esb_debug("stage %d, timeout %" PRIi64 "\n", d->stage, timeout);
 
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 2/9] rtl8139: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
  2015-08-27 19:32 ` [Qemu-devel] [PATCH v3 1/9] i6300esb: remove muldiv64() Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-08-28 14:12   ` Stefan Hajnoczi
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 3/9] pcnet: " Laurent Vivier
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Michael S . Tsirkin, Paolo Bonzini

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds, by
doing something like:

    y = muldiv64(x, get_ticks_per_sec(), PCI_FREQUENCY)

where x is the number of device ticks and y the number of system ticks.

y is used as nanoseconds in timer functions,
it works because 1 tick is 1 nanosecond.
(get_ticks_per_sec() is 10^9)

But as PCI frequency is 33 MHz, we can also do:

    y = x * 30; /* 33 MHz PCI period is 30 ns */

Which is much more simple.

This implies a 33.333333 MHz PCI frequency,
but this is correct.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v3: part of "PCI: remove muldiv64()"
 hw/net/rtl8139.c     | 14 ++++++--------
 tests/rtl8139-test.c |  2 +-
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index edbb61c..1e119e6 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -63,7 +63,7 @@
 /* debug RTL8139 card */
 //#define DEBUG_RTL8139 1
 
-#define PCI_FREQUENCY 33000000L
+#define PCI_PERIOD 30    /* 30 ns period = 33.333333 Mhz frequency */
 
 #define SET_MASKED(input, mask, curr) \
     ( ( (input) & ~(mask) ) | ( (curr) & (mask) ) )
@@ -2881,8 +2881,7 @@ static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
 
 static void rtl8139_set_next_tctr_time(RTL8139State *s)
 {
-    const uint64_t ns_per_period =
-        muldiv64(0x100000000LL, get_ticks_per_sec(), PCI_FREQUENCY);
+    const uint64_t ns_per_period = (uint64_t)PCI_PERIOD << 32;
 
     DPRINTF("entered rtl8139_set_next_tctr_time\n");
 
@@ -2900,7 +2899,7 @@ static void rtl8139_set_next_tctr_time(RTL8139State *s)
     if (!s->TimerInt) {
         timer_del(s->timer);
     } else {
-        uint64_t delta = muldiv64(s->TimerInt, get_ticks_per_sec(), PCI_FREQUENCY);
+        uint64_t delta = (uint64_t)s->TimerInt * PCI_PERIOD;
         if (s->TCTR_base + delta <= qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
             delta += ns_per_period;
         }
@@ -3174,8 +3173,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
             break;
 
         case Timer:
-            ret = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - s->TCTR_base,
-                           PCI_FREQUENCY, get_ticks_per_sec());
+            ret = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - s->TCTR_base) /
+                  PCI_PERIOD;
             DPRINTF("TCTR Timer read val=0x%08x\n", ret);
             break;
 
@@ -3269,8 +3268,7 @@ static void rtl8139_pre_save(void *opaque)
     int64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
     /* for migration to older versions */
-    s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
-                       get_ticks_per_sec());
+    s->TCTR = (current_time - s->TCTR_base) / PCI_PERIOD;
     s->rtl8139_mmio_io_addr_dummy = 0;
 }
 
diff --git a/tests/rtl8139-test.c b/tests/rtl8139-test.c
index e749be3..ba62851 100644
--- a/tests/rtl8139-test.c
+++ b/tests/rtl8139-test.c
@@ -20,7 +20,7 @@ static void nop(void)
 {
 }
 
-#define CLK 33000000
+#define CLK 33333333
 
 static QPCIBus *pcibus;
 static QPCIDevice *dev;
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 3/9] pcnet: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
  2015-08-27 19:32 ` [Qemu-devel] [PATCH v3 1/9] i6300esb: remove muldiv64() Laurent Vivier
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 2/9] rtl8139: " Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-08-28 14:12   ` Stefan Hajnoczi
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 4/9] mips: " Laurent Vivier
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Michael S . Tsirkin, Paolo Bonzini

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds, by
doing something like:

    y = muldiv64(x, get_ticks_per_sec(), PCI_FREQUENCY)

where x is the number of device ticks and y the number of system ticks.

y is used as nanoseconds in timer functions,
it works because 1 tick is 1 nanosecond.
(get_ticks_per_sec() is 10^9)

But as PCI frequency is 33 MHz, we can also do:

    y = x * 30; /* 33 MHz PCI period is 30 ns */

Which is much more simple.

This implies a 33.333333 MHz PCI frequency,
but this is correct.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v3: part of "PCI: remove muldiv64()"
 hw/net/pcnet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 3437376..0eb3cc4 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -670,8 +670,7 @@ static inline hwaddr pcnet_rdra_addr(PCNetState *s, int idx)
 static inline int64_t pcnet_get_next_poll_time(PCNetState *s, int64_t current_time)
 {
     int64_t next_time = current_time +
-        muldiv64(65536 - (CSR_SPND(s) ? 0 : CSR_POLL(s)),
-                 get_ticks_per_sec(), 33000000L);
+                        (65536 - (CSR_SPND(s) ? 0 : CSR_POLL(s))) * 30;
     if (next_time <= current_time)
         next_time = current_time + 1;
     return next_time;
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 4/9] mips: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
                   ` (2 preceding siblings ...)
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 3/9] pcnet: " Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-09-08 12:54   ` Laurent Vivier
  2015-09-08 13:42   ` Leon Alrae
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 5/9] openrisc: " Laurent Vivier
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Aurelien Jarno, Leon Alrae

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds, by
doing something like:

    y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)

where x is the number of device ticks and y the number of system ticks.

y is used as nanoseconds in timer functions,
it works because 1 tick is 1 nanosecond.
(get_ticks_per_sec() is 10^9)

But as MIPS timer frequency is 100 MHz, we can also do:

    y = x * 10; /* 100 MHz period is 10 ns */

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/mips/cputimer.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c
index 577c9ae..a74ae5c 100644
--- a/hw/mips/cputimer.c
+++ b/hw/mips/cputimer.c
@@ -25,7 +25,7 @@
 #include "qemu/timer.h"
 #include "sysemu/kvm.h"
 
-#define TIMER_FREQ	100 * 1000 * 1000
+#define TIMER_PERIOD 10 /* 10 ns period for 100 Mhz frequency */
 
 /* XXX: do not use a global */
 uint32_t cpu_mips_get_random (CPUMIPSState *env)
@@ -49,9 +49,8 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
     uint32_t wait;
 
     now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    wait = env->CP0_Compare - env->CP0_Count -
-	    (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
-    next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
+    wait = env->CP0_Compare - env->CP0_Count - (uint32_t)(now / TIMER_PERIOD);
+    next = now + (uint64_t)wait * TIMER_PERIOD;
     timer_mod(env->timer, next);
 }
 
@@ -79,8 +78,7 @@ uint32_t cpu_mips_get_count (CPUMIPSState *env)
             cpu_mips_timer_expire(env);
         }
 
-        return env->CP0_Count +
-            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
+        return env->CP0_Count + (uint32_t)(now / TIMER_PERIOD);
     }
 }
 
@@ -95,9 +93,8 @@ void cpu_mips_store_count (CPUMIPSState *env, uint32_t count)
         env->CP0_Count = count;
     else {
         /* Store new count register */
-        env->CP0_Count =
-            count - (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                                       TIMER_FREQ, get_ticks_per_sec());
+        env->CP0_Count = count -
+               (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / TIMER_PERIOD);
         /* Update timer timer */
         cpu_mips_timer_update(env);
     }
@@ -121,8 +118,8 @@ void cpu_mips_start_count(CPUMIPSState *env)
 void cpu_mips_stop_count(CPUMIPSState *env)
 {
     /* Store the current value */
-    env->CP0_Count += (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                                         TIMER_FREQ, get_ticks_per_sec());
+    env->CP0_Count += (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
+                                 TIMER_PERIOD);
 }
 
 static void mips_timer_cb (void *opaque)
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 5/9] openrisc: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
                   ` (3 preceding siblings ...)
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 4/9] mips: " Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-09-08 12:54   ` Laurent Vivier
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64() Laurent Vivier
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Jia Liu

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds, by
doing something like:

    y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)

where x is the number of device ticks and y the number of system ticks.

y is used as nanoseconds in timer functions,
it works because 1 tick is 1 nanosecond.
(get_ticks_per_sec() is 10^9)

But as openrisc timer frequency is 20 MHz, we can also do:

    y = x * 50; /* 20 MHz period is 50 ns */

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/openrisc/cputimer.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
index 9c54945..560cb91 100644
--- a/hw/openrisc/cputimer.c
+++ b/hw/openrisc/cputimer.c
@@ -22,7 +22,7 @@
 #include "hw/hw.h"
 #include "qemu/timer.h"
 
-#define TIMER_FREQ    (20 * 1000 * 1000)    /* 20MHz */
+#define TIMER_PERIOD 50 /* 50 ns period for 20 MHz timer */
 
 /* The time when TTCR changes */
 static uint64_t last_clk;
@@ -36,8 +36,7 @@ void cpu_openrisc_count_update(OpenRISCCPU *cpu)
         return;
     }
     now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    cpu->env.ttcr += (uint32_t)muldiv64(now - last_clk, TIMER_FREQ,
-                                        get_ticks_per_sec());
+    cpu->env.ttcr += (uint32_t)((now - last_clk) / TIMER_PERIOD);
     last_clk = now;
 }
 
@@ -59,7 +58,7 @@ void cpu_openrisc_timer_update(OpenRISCCPU *cpu)
     } else {
         wait = (cpu->env.ttmr & TTMR_TP) - (cpu->env.ttcr & TTMR_TP);
     }
-    next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
+    next = now + (uint64_t)wait * TIMER_PERIOD;
     timer_mod(cpu->env.timer, next);
 }
 
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
                   ` (4 preceding siblings ...)
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 5/9] openrisc: " Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-08-27 20:23   ` Peter Crosthwaite
  2015-09-01 11:17   ` Peter Maydell
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64() Laurent Vivier
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell, qemu-trivial

muldiv64() is used to convert microseconds into CPU ticks.

But it is not clear and not commented. This patch uses macro
to clearly identify what is used: time, CPU frequency and ticks.
For an elapsed time and a given frequency, we compute how many ticks
 we have.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v2: replace "arm: remove muldiv64()"
 target-arm/helper.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7df1f06..4455761 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -12,6 +12,8 @@
 #include <zlib.h> /* For crc32 */
 #include "exec/semihost.h"
 
+#define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
+
 #ifndef CONFIG_USER_ONLY
 static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
                                  int access_type, ARMMMUIdx mmu_idx,
@@ -678,8 +680,8 @@ void pmccntr_sync(CPUARMState *env)
 {
     uint64_t temp_ticks;
 
-    temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
-                          get_ticks_per_sec(), 1000000);
+    temp_ticks = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
 
     if (env->cp15.c9_pmcr & PMCRD) {
         /* Increment once every 64 processor clock cycles */
@@ -717,8 +719,8 @@ static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
         return env->cp15.c15_ccnt;
     }
 
-    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
-                           get_ticks_per_sec(), 1000000);
+    total_ticks = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+                           ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
 
     if (env->cp15.c9_pmcr & PMCRD) {
         /* Increment once every 64 processor clock cycles */
@@ -738,8 +740,8 @@ static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
         return;
     }
 
-    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
-                           get_ticks_per_sec(), 1000000);
+    total_ticks = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+                           ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
 
     if (env->cp15.c9_pmcr & PMCRD) {
         /* Increment once every 64 processor clock cycles */
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
                   ` (5 preceding siblings ...)
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64() Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-09-08 12:55   ` Laurent Vivier
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 8/9] bt: " Laurent Vivier
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Paolo Bonzini, qemu-trivial

hpet defines a clock period in femtoseconds but
then converts it to nanoseconds to use the internal
timers.

We can define the period in nanoseconds and use it
directly, this allows to remove muldiv64().

We only need to convert the period to femtoseconds
to put it in internal hpet capability register.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/timer/hpet.c         | 6 +++---
 include/hw/timer/hpet.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 2bb6221..3037bef 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -126,12 +126,12 @@ static uint32_t hpet_time_after64(uint64_t a, uint64_t b)
 
 static uint64_t ticks_to_ns(uint64_t value)
 {
-    return (muldiv64(value, HPET_CLK_PERIOD, FS_PER_NS));
+    return value * HPET_CLK_PERIOD;
 }
 
 static uint64_t ns_to_ticks(uint64_t value)
 {
-    return (muldiv64(value, FS_PER_NS, HPET_CLK_PERIOD));
+    return value / HPET_CLK_PERIOD;
 }
 
 static uint64_t hpet_fixup_reg(uint64_t new, uint64_t old, uint64_t mask)
@@ -758,7 +758,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
     /* 64-bit main counter; LegacyReplacementRoute. */
     s->capability = 0x8086a001ULL;
     s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
-    s->capability |= ((HPET_CLK_PERIOD) << 32);
+    s->capability |= ((uint64_t)(HPET_CLK_PERIOD * FS_PER_NS) << 32);
 
     qdev_init_gpio_in(dev, hpet_handle_legacy_irq, 2);
     qdev_init_gpio_out(dev, &s->pit_enabled, 1);
diff --git a/include/hw/timer/hpet.h b/include/hw/timer/hpet.h
index 773953b..d872909 100644
--- a/include/hw/timer/hpet.h
+++ b/include/hw/timer/hpet.h
@@ -16,9 +16,9 @@
 #include "qom/object.h"
 
 #define HPET_BASE               0xfed00000
-#define HPET_CLK_PERIOD         10000000ULL /* 10000000 femtoseconds == 10ns*/
+#define HPET_CLK_PERIOD         10 /* 10 ns*/
 
-#define FS_PER_NS 1000000
+#define FS_PER_NS 1000000       /* 1000000 femtoseconds == 1 ns */
 #define HPET_MIN_TIMERS         3
 #define HPET_MAX_TIMERS         32
 
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 8/9] bt: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
                   ` (6 preceding siblings ...)
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64() Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-09-08 12:55   ` Laurent Vivier
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 9/9] net: " Laurent Vivier
  2015-09-24 16:21 ` [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Paolo Bonzini, qemu-trivial

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds.

As get_ticks_per_sec() is 10^9,

    a = muldiv64(b, get_ticks_per_sec(), 100);
    y = muldiv64(x, get_ticks_per_sec(), 1000000);

can be converted to

    a = b * 10000000;
    y = x * 1000;

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/bt/hci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/bt/hci.c b/hw/bt/hci.c
index 7ea3dc6..585ee2e 100644
--- a/hw/bt/hci.c
+++ b/hw/bt/hci.c
@@ -595,7 +595,7 @@ static void bt_hci_inquiry_result(struct bt_hci_s *hci,
 static void bt_hci_mod_timer_1280ms(QEMUTimer *timer, int period)
 {
     timer_mod(timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-                   muldiv64(period << 7, get_ticks_per_sec(), 100));
+                     (uint64_t)(period << 7) * 10000000);
 }
 
 static void bt_hci_inquiry_start(struct bt_hci_s *hci, int length)
@@ -1099,7 +1099,7 @@ static int bt_hci_mode_change(struct bt_hci_s *hci, uint16_t handle,
     bt_hci_event_status(hci, HCI_SUCCESS);
 
     timer_mod(link->acl_mode_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-                   muldiv64(interval * 625, get_ticks_per_sec(), 1000000));
+                                    ((uint64_t)interval * 625) * 1000);
     bt_hci_lmp_mode_change_master(hci, link->link, mode, interval);
 
     return 0;
-- 
2.1.0

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

* [Qemu-devel] [PATCH v3 9/9] net: remove muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
                   ` (7 preceding siblings ...)
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 8/9] bt: " Laurent Vivier
@ 2015-08-27 19:33 ` Laurent Vivier
  2015-08-28 14:12   ` Stefan Hajnoczi
  2015-09-24 16:21 ` [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-08-27 19:33 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Jason Wang, qemu-trivial

muldiv64() is used to convert nanoseconds to microseconds.

    x = muldiv64(qemu_clock_get_ns(..), 1000000, get_ticks_per_sec());

As  get_ticks_per_sec() is 10^9, it can be replaced by:

    x = qemu_clock_get_us(..);

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v2: use qemu_clock_get_us() instead of qemu_clock_get_ns()/1000
 net/dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dump.c b/net/dump.c
index 02c8064..08259af 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -69,7 +69,7 @@ static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         return size;
     }
 
-    ts = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 1000000, get_ticks_per_sec());
+    ts = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
     caplen = size > s->pcap_caplen ? s->pcap_caplen : size;
 
     hdr.ts.tv_sec = ts / 1000000 + s->start_ts;
-- 
2.1.0

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

* Re: [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64() Laurent Vivier
@ 2015-08-27 20:23   ` Peter Crosthwaite
  2015-09-01 11:17   ` Peter Maydell
  1 sibling, 0 replies; 27+ messages in thread
From: Peter Crosthwaite @ 2015-08-27 20:23 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: qemu-trivial, Peter Maydell, qemu-devel@nongnu.org Developers

On Thu, Aug 27, 2015 at 12:33 PM, Laurent Vivier <lvivier@redhat.com> wrote:
> muldiv64() is used to convert microseconds into CPU ticks.
>
> But it is not clear and not commented. This patch uses macro
> to clearly identify what is used: time, CPU frequency and ticks.
> For an elapsed time and a given frequency, we compute how many ticks
>  we have.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

> ---
> v2: replace "arm: remove muldiv64()"
>  target-arm/helper.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 7df1f06..4455761 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -12,6 +12,8 @@
>  #include <zlib.h> /* For crc32 */
>  #include "exec/semihost.h"
>
> +#define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
> +
>  #ifndef CONFIG_USER_ONLY
>  static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
>                                   int access_type, ARMMMUIdx mmu_idx,
> @@ -678,8 +680,8 @@ void pmccntr_sync(CPUARMState *env)
>  {
>      uint64_t temp_ticks;
>
> -    temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
> -                          get_ticks_per_sec(), 1000000);
> +    temp_ticks = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> +                          ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
>
>      if (env->cp15.c9_pmcr & PMCRD) {
>          /* Increment once every 64 processor clock cycles */
> @@ -717,8 +719,8 @@ static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>          return env->cp15.c15_ccnt;
>      }
>
> -    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
> -                           get_ticks_per_sec(), 1000000);
> +    total_ticks = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> +                           ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
>
>      if (env->cp15.c9_pmcr & PMCRD) {
>          /* Increment once every 64 processor clock cycles */
> @@ -738,8 +740,8 @@ static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>          return;
>      }
>
> -    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
> -                           get_ticks_per_sec(), 1000000);
> +    total_ticks = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> +                           ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
>
>      if (env->cp15.c9_pmcr & PMCRD) {
>          /* Increment once every 64 processor clock cycles */
> --
> 2.1.0
>
>

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

* Re: [Qemu-devel] [PATCH v3 9/9] net: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 9/9] net: " Laurent Vivier
@ 2015-08-28 14:12   ` Stefan Hajnoczi
  0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2015-08-28 14:12 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: qemu-trivial, Jason Wang, qemu-devel

On Thu, Aug 27, 2015 at 09:33:07PM +0200, Laurent Vivier wrote:
> muldiv64() is used to convert nanoseconds to microseconds.
> 
>     x = muldiv64(qemu_clock_get_ns(..), 1000000, get_ticks_per_sec());
> 
> As  get_ticks_per_sec() is 10^9, it can be replaced by:
> 
>     x = qemu_clock_get_us(..);
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2: use qemu_clock_get_us() instead of qemu_clock_get_ns()/1000
>  net/dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

* Re: [Qemu-devel] [PATCH v3 2/9] rtl8139: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 2/9] rtl8139: " Laurent Vivier
@ 2015-08-28 14:12   ` Stefan Hajnoczi
  0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2015-08-28 14:12 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Paolo Bonzini, qemu-devel, Michael S . Tsirkin

On Thu, Aug 27, 2015 at 09:33:00PM +0200, Laurent Vivier wrote:
> Originally, timers were ticks based, and it made sense to
> add ticks to current time to know when to trigger an alarm.
> 
> But since commit:
> 
> 7447545 change all other clock references to use nanosecond resolution accessors
> 
> All timers use nanoseconds and we need to convert ticks to nanoseconds, by
> doing something like:
> 
>     y = muldiv64(x, get_ticks_per_sec(), PCI_FREQUENCY)
> 
> where x is the number of device ticks and y the number of system ticks.
> 
> y is used as nanoseconds in timer functions,
> it works because 1 tick is 1 nanosecond.
> (get_ticks_per_sec() is 10^9)
> 
> But as PCI frequency is 33 MHz, we can also do:
> 
>     y = x * 30; /* 33 MHz PCI period is 30 ns */
> 
> Which is much more simple.
> 
> This implies a 33.333333 MHz PCI frequency,
> but this is correct.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v3: part of "PCI: remove muldiv64()"
>  hw/net/rtl8139.c     | 14 ++++++--------
>  tests/rtl8139-test.c |  2 +-
>  2 files changed, 7 insertions(+), 9 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

* Re: [Qemu-devel] [PATCH v3 3/9] pcnet: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 3/9] pcnet: " Laurent Vivier
@ 2015-08-28 14:12   ` Stefan Hajnoczi
  0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2015-08-28 14:12 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Paolo Bonzini, qemu-devel, Michael S . Tsirkin

On Thu, Aug 27, 2015 at 09:33:01PM +0200, Laurent Vivier wrote:
> Originally, timers were ticks based, and it made sense to
> add ticks to current time to know when to trigger an alarm.
> 
> But since commit:
> 
> 7447545 change all other clock references to use nanosecond resolution accessors
> 
> All timers use nanoseconds and we need to convert ticks to nanoseconds, by
> doing something like:
> 
>     y = muldiv64(x, get_ticks_per_sec(), PCI_FREQUENCY)
> 
> where x is the number of device ticks and y the number of system ticks.
> 
> y is used as nanoseconds in timer functions,
> it works because 1 tick is 1 nanosecond.
> (get_ticks_per_sec() is 10^9)
> 
> But as PCI frequency is 33 MHz, we can also do:
> 
>     y = x * 30; /* 33 MHz PCI period is 30 ns */
> 
> Which is much more simple.
> 
> This implies a 33.333333 MHz PCI frequency,
> but this is correct.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v3: part of "PCI: remove muldiv64()"
>  hw/net/pcnet.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

* Re: [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64() Laurent Vivier
  2015-08-27 20:23   ` Peter Crosthwaite
@ 2015-09-01 11:17   ` Peter Maydell
  2015-09-01 11:23     ` Laurent Vivier
  1 sibling, 1 reply; 27+ messages in thread
From: Peter Maydell @ 2015-09-01 11:17 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: QEMU Trivial, QEMU Developers

On 27 August 2015 at 20:33, Laurent Vivier <lvivier@redhat.com> wrote:
> muldiv64() is used to convert microseconds into CPU ticks.
>
> But it is not clear and not commented. This patch uses macro
> to clearly identify what is used: time, CPU frequency and ticks.
> For an elapsed time and a given frequency, we compute how many ticks
>  we have.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2: replace "arm: remove muldiv64()"
>  target-arm/helper.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 7df1f06..4455761 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -12,6 +12,8 @@
>  #include <zlib.h> /* For crc32 */
>  #include "exec/semihost.h"
>
> +#define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */

LL suffix for large constants, please.

Otherwise
Acked-by: Peter Maydell <peter.maydell@linaro.org>

(assuming you're planning to put this somewhere other than the
target-arm tree).

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64()
  2015-09-01 11:17   ` Peter Maydell
@ 2015-09-01 11:23     ` Laurent Vivier
  2015-09-01 11:30       ` Peter Maydell
  0 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-09-01 11:23 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Trivial, QEMU Developers



On 01/09/2015 13:17, Peter Maydell wrote:
> On 27 August 2015 at 20:33, Laurent Vivier <lvivier@redhat.com> wrote:
>> muldiv64() is used to convert microseconds into CPU ticks.
>>
>> But it is not clear and not commented. This patch uses macro
>> to clearly identify what is used: time, CPU frequency and ticks.
>> For an elapsed time and a given frequency, we compute how many ticks
>>  we have.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> v2: replace "arm: remove muldiv64()"
>>  target-arm/helper.c | 14 ++++++++------
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 7df1f06..4455761 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -12,6 +12,8 @@
>>  #include <zlib.h> /* For crc32 */
>>  #include "exec/semihost.h"
>>
>> +#define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
> 
> LL suffix for large constants, please.

In fact, I didn't put the LL suffix to not force the use of a 64bit on a
32bit machines. Moreover in muldiv64() it is used as a 32bit value.

But if you think it is better, I will. Have I to resend the whole series
or only this patch ? Perhaps the commiter can edit it ?

> 
> Otherwise
> Acked-by: Peter Maydell <peter.maydell@linaro.org>
> 
> (assuming you're planning to put this somewhere other than the
> target-arm tree).
> 
> thanks
> -- PMM
> 

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

* Re: [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64()
  2015-09-01 11:23     ` Laurent Vivier
@ 2015-09-01 11:30       ` Peter Maydell
  0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2015-09-01 11:30 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: QEMU Trivial, QEMU Developers

On 1 September 2015 at 12:23, Laurent Vivier <lvivier@redhat.com> wrote:
>
>
> On 01/09/2015 13:17, Peter Maydell wrote:
>> On 27 August 2015 at 20:33, Laurent Vivier <lvivier@redhat.com> wrote:
>>> +#define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */
>>
>> LL suffix for large constants, please.
>
> In fact, I didn't put the LL suffix to not force the use of a 64bit on a
> 32bit machines. Moreover in muldiv64() it is used as a 32bit value.
>
> But if you think it is better, I will. Have I to resend the whole series
> or only this patch ? Perhaps the commiter can edit it ?

Oh, you're right, it's a 32-bit value. I take back my suggestion.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v3 4/9] mips: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 4/9] mips: " Laurent Vivier
@ 2015-09-08 12:54   ` Laurent Vivier
  2015-09-08 13:42   ` Leon Alrae
  1 sibling, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2015-09-08 12:54 UTC (permalink / raw)
  To: qemu-devel, Aurelien Jarno, Leon Alrae

ping ?

On 27/08/2015 21:33, Laurent Vivier wrote:
> Originally, timers were ticks based, and it made sense to
> add ticks to current time to know when to trigger an alarm.
> 
> But since commit:
> 
> 7447545 change all other clock references to use nanosecond resolution accessors
> 
> All timers use nanoseconds and we need to convert ticks to nanoseconds, by
> doing something like:
> 
>     y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)
> 
> where x is the number of device ticks and y the number of system ticks.
> 
> y is used as nanoseconds in timer functions,
> it works because 1 tick is 1 nanosecond.
> (get_ticks_per_sec() is 10^9)
> 
> But as MIPS timer frequency is 100 MHz, we can also do:
> 
>     y = x * 10; /* 100 MHz period is 10 ns */
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/mips/cputimer.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c
> index 577c9ae..a74ae5c 100644
> --- a/hw/mips/cputimer.c
> +++ b/hw/mips/cputimer.c
> @@ -25,7 +25,7 @@
>  #include "qemu/timer.h"
>  #include "sysemu/kvm.h"
>  
> -#define TIMER_FREQ	100 * 1000 * 1000
> +#define TIMER_PERIOD 10 /* 10 ns period for 100 Mhz frequency */
>  
>  /* XXX: do not use a global */
>  uint32_t cpu_mips_get_random (CPUMIPSState *env)
> @@ -49,9 +49,8 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
>      uint32_t wait;
>  
>      now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -    wait = env->CP0_Compare - env->CP0_Count -
> -	    (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> -    next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
> +    wait = env->CP0_Compare - env->CP0_Count - (uint32_t)(now / TIMER_PERIOD);
> +    next = now + (uint64_t)wait * TIMER_PERIOD;
>      timer_mod(env->timer, next);
>  }
>  
> @@ -79,8 +78,7 @@ uint32_t cpu_mips_get_count (CPUMIPSState *env)
>              cpu_mips_timer_expire(env);
>          }
>  
> -        return env->CP0_Count +
> -            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> +        return env->CP0_Count + (uint32_t)(now / TIMER_PERIOD);
>      }
>  }
>  
> @@ -95,9 +93,8 @@ void cpu_mips_store_count (CPUMIPSState *env, uint32_t count)
>          env->CP0_Count = count;
>      else {
>          /* Store new count register */
> -        env->CP0_Count =
> -            count - (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                       TIMER_FREQ, get_ticks_per_sec());
> +        env->CP0_Count = count -
> +               (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / TIMER_PERIOD);
>          /* Update timer timer */
>          cpu_mips_timer_update(env);
>      }
> @@ -121,8 +118,8 @@ void cpu_mips_start_count(CPUMIPSState *env)
>  void cpu_mips_stop_count(CPUMIPSState *env)
>  {
>      /* Store the current value */
> -    env->CP0_Count += (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                         TIMER_FREQ, get_ticks_per_sec());
> +    env->CP0_Count += (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
> +                                 TIMER_PERIOD);
>  }
>  
>  static void mips_timer_cb (void *opaque)
> 

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

* Re: [Qemu-devel] [PATCH v3 5/9] openrisc: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 5/9] openrisc: " Laurent Vivier
@ 2015-09-08 12:54   ` Laurent Vivier
  0 siblings, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2015-09-08 12:54 UTC (permalink / raw)
  To: qemu-devel, Jia Liu

ping ?

On 27/08/2015 21:33, Laurent Vivier wrote:
> Originally, timers were ticks based, and it made sense to
> add ticks to current time to know when to trigger an alarm.
> 
> But since commit:
> 
> 7447545 change all other clock references to use nanosecond resolution accessors
> 
> All timers use nanoseconds and we need to convert ticks to nanoseconds, by
> doing something like:
> 
>     y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)
> 
> where x is the number of device ticks and y the number of system ticks.
> 
> y is used as nanoseconds in timer functions,
> it works because 1 tick is 1 nanosecond.
> (get_ticks_per_sec() is 10^9)
> 
> But as openrisc timer frequency is 20 MHz, we can also do:
> 
>     y = x * 50; /* 20 MHz period is 50 ns */
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/openrisc/cputimer.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
> index 9c54945..560cb91 100644
> --- a/hw/openrisc/cputimer.c
> +++ b/hw/openrisc/cputimer.c
> @@ -22,7 +22,7 @@
>  #include "hw/hw.h"
>  #include "qemu/timer.h"
>  
> -#define TIMER_FREQ    (20 * 1000 * 1000)    /* 20MHz */
> +#define TIMER_PERIOD 50 /* 50 ns period for 20 MHz timer */
>  
>  /* The time when TTCR changes */
>  static uint64_t last_clk;
> @@ -36,8 +36,7 @@ void cpu_openrisc_count_update(OpenRISCCPU *cpu)
>          return;
>      }
>      now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -    cpu->env.ttcr += (uint32_t)muldiv64(now - last_clk, TIMER_FREQ,
> -                                        get_ticks_per_sec());
> +    cpu->env.ttcr += (uint32_t)((now - last_clk) / TIMER_PERIOD);
>      last_clk = now;
>  }
>  
> @@ -59,7 +58,7 @@ void cpu_openrisc_timer_update(OpenRISCCPU *cpu)
>      } else {
>          wait = (cpu->env.ttmr & TTMR_TP) - (cpu->env.ttcr & TTMR_TP);
>      }
> -    next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
> +    next = now + (uint64_t)wait * TIMER_PERIOD;
>      timer_mod(cpu->env.timer, next);
>  }
>  
> 

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

* Re: [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64() Laurent Vivier
@ 2015-09-08 12:55   ` Laurent Vivier
  2015-09-08 12:58     ` Paolo Bonzini
  0 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-09-08 12:55 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Paolo Bonzini, qemu-trivial

ping ?

On 27/08/2015 21:33, Laurent Vivier wrote:
> hpet defines a clock period in femtoseconds but
> then converts it to nanoseconds to use the internal
> timers.
> 
> We can define the period in nanoseconds and use it
> directly, this allows to remove muldiv64().
> 
> We only need to convert the period to femtoseconds
> to put it in internal hpet capability register.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/timer/hpet.c         | 6 +++---
>  include/hw/timer/hpet.h | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 2bb6221..3037bef 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -126,12 +126,12 @@ static uint32_t hpet_time_after64(uint64_t a, uint64_t b)
>  
>  static uint64_t ticks_to_ns(uint64_t value)
>  {
> -    return (muldiv64(value, HPET_CLK_PERIOD, FS_PER_NS));
> +    return value * HPET_CLK_PERIOD;
>  }
>  
>  static uint64_t ns_to_ticks(uint64_t value)
>  {
> -    return (muldiv64(value, FS_PER_NS, HPET_CLK_PERIOD));
> +    return value / HPET_CLK_PERIOD;
>  }
>  
>  static uint64_t hpet_fixup_reg(uint64_t new, uint64_t old, uint64_t mask)
> @@ -758,7 +758,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
>      /* 64-bit main counter; LegacyReplacementRoute. */
>      s->capability = 0x8086a001ULL;
>      s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
> -    s->capability |= ((HPET_CLK_PERIOD) << 32);
> +    s->capability |= ((uint64_t)(HPET_CLK_PERIOD * FS_PER_NS) << 32);
>  
>      qdev_init_gpio_in(dev, hpet_handle_legacy_irq, 2);
>      qdev_init_gpio_out(dev, &s->pit_enabled, 1);
> diff --git a/include/hw/timer/hpet.h b/include/hw/timer/hpet.h
> index 773953b..d872909 100644
> --- a/include/hw/timer/hpet.h
> +++ b/include/hw/timer/hpet.h
> @@ -16,9 +16,9 @@
>  #include "qom/object.h"
>  
>  #define HPET_BASE               0xfed00000
> -#define HPET_CLK_PERIOD         10000000ULL /* 10000000 femtoseconds == 10ns*/
> +#define HPET_CLK_PERIOD         10 /* 10 ns*/
>  
> -#define FS_PER_NS 1000000
> +#define FS_PER_NS 1000000       /* 1000000 femtoseconds == 1 ns */
>  #define HPET_MIN_TIMERS         3
>  #define HPET_MAX_TIMERS         32
>  
> 

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

* Re: [Qemu-devel] [PATCH v3 8/9] bt: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 8/9] bt: " Laurent Vivier
@ 2015-09-08 12:55   ` Laurent Vivier
  2015-09-08 12:57     ` Paolo Bonzini
  0 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-09-08 12:55 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi, Paolo Bonzini, qemu-trivial

ping ?

On 27/08/2015 21:33, Laurent Vivier wrote:
> Originally, timers were ticks based, and it made sense to
> add ticks to current time to know when to trigger an alarm.
> 
> But since commit:
> 
> 7447545 change all other clock references to use nanosecond resolution accessors
> 
> All timers use nanoseconds and we need to convert ticks to nanoseconds.
> 
> As get_ticks_per_sec() is 10^9,
> 
>     a = muldiv64(b, get_ticks_per_sec(), 100);
>     y = muldiv64(x, get_ticks_per_sec(), 1000000);
> 
> can be converted to
> 
>     a = b * 10000000;
>     y = x * 1000;
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/bt/hci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/bt/hci.c b/hw/bt/hci.c
> index 7ea3dc6..585ee2e 100644
> --- a/hw/bt/hci.c
> +++ b/hw/bt/hci.c
> @@ -595,7 +595,7 @@ static void bt_hci_inquiry_result(struct bt_hci_s *hci,
>  static void bt_hci_mod_timer_1280ms(QEMUTimer *timer, int period)
>  {
>      timer_mod(timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> -                   muldiv64(period << 7, get_ticks_per_sec(), 100));
> +                     (uint64_t)(period << 7) * 10000000);
>  }
>  
>  static void bt_hci_inquiry_start(struct bt_hci_s *hci, int length)
> @@ -1099,7 +1099,7 @@ static int bt_hci_mode_change(struct bt_hci_s *hci, uint16_t handle,
>      bt_hci_event_status(hci, HCI_SUCCESS);
>  
>      timer_mod(link->acl_mode_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> -                   muldiv64(interval * 625, get_ticks_per_sec(), 1000000));
> +                                    ((uint64_t)interval * 625) * 1000);
>      bt_hci_lmp_mode_change_master(hci, link->link, mode, interval);
>  
>      return 0;
> 

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

* Re: [Qemu-devel] [PATCH v3 8/9] bt: remove muldiv64()
  2015-09-08 12:55   ` Laurent Vivier
@ 2015-09-08 12:57     ` Paolo Bonzini
  0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2015-09-08 12:57 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: qemu-trivial, qemu-devel, Stefan Hajnoczi

> ping ?

Not really the most alive part of QEMU. :-)

> On 27/08/2015 21:33, Laurent Vivier wrote:
> > Originally, timers were ticks based, and it made sense to
> > add ticks to current time to know when to trigger an alarm.
> > 
> > But since commit:
> > 
> > 7447545 change all other clock references to use nanosecond resolution
> > accessors
> > 
> > All timers use nanoseconds and we need to convert ticks to nanoseconds.
> > 
> > As get_ticks_per_sec() is 10^9,
> > 
> >     a = muldiv64(b, get_ticks_per_sec(), 100);
> >     y = muldiv64(x, get_ticks_per_sec(), 1000000);
> > 
> > can be converted to
> > 
> >     a = b * 10000000;
> >     y = x * 1000;
> > 
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> >  hw/bt/hci.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/bt/hci.c b/hw/bt/hci.c
> > index 7ea3dc6..585ee2e 100644
> > --- a/hw/bt/hci.c
> > +++ b/hw/bt/hci.c
> > @@ -595,7 +595,7 @@ static void bt_hci_inquiry_result(struct bt_hci_s *hci,
> >  static void bt_hci_mod_timer_1280ms(QEMUTimer *timer, int period)
> >  {
> >      timer_mod(timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> > -                   muldiv64(period << 7, get_ticks_per_sec(), 100));
> > +                     (uint64_t)(period << 7) * 10000000);
> >  }
> >  
> >  static void bt_hci_inquiry_start(struct bt_hci_s *hci, int length)
> > @@ -1099,7 +1099,7 @@ static int bt_hci_mode_change(struct bt_hci_s *hci,
> > uint16_t handle,
> >      bt_hci_event_status(hci, HCI_SUCCESS);
> >  
> >      timer_mod(link->acl_mode_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
> >      +
> > -                   muldiv64(interval * 625, get_ticks_per_sec(),
> > 1000000));
> > +                                    ((uint64_t)interval * 625) * 1000);
> >      bt_hci_lmp_mode_change_master(hci, link->link, mode, interval);
> >  
> >      return 0;
> > 
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

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

* Re: [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64()
  2015-09-08 12:55   ` Laurent Vivier
@ 2015-09-08 12:58     ` Paolo Bonzini
  0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2015-09-08 12:58 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: qemu-trivial, qemu-devel, Stefan Hajnoczi



----- Messaggio originale -----
> Da: "Laurent Vivier" <lvivier@redhat.com>
> A: qemu-devel@nongnu.org, "Stefan Hajnoczi" <stefanha@redhat.com>, "Paolo Bonzini" <pbonzini@redhat.com>,
> qemu-trivial@nongnu.org
> Inviato: Martedì, 8 settembre 2015 14:55:25
> Oggetto: Re: [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64()
> 
> ping ?
> 
> On 27/08/2015 21:33, Laurent Vivier wrote:
> > hpet defines a clock period in femtoseconds but
> > then converts it to nanoseconds to use the internal
> > timers.
> > 
> > We can define the period in nanoseconds and use it
> > directly, this allows to remove muldiv64().
> > 
> > We only need to convert the period to femtoseconds
> > to put it in internal hpet capability register.
> > 
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> >  hw/timer/hpet.c         | 6 +++---
> >  include/hw/timer/hpet.h | 4 ++--
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> > index 2bb6221..3037bef 100644
> > --- a/hw/timer/hpet.c
> > +++ b/hw/timer/hpet.c
> > @@ -126,12 +126,12 @@ static uint32_t hpet_time_after64(uint64_t a,
> > uint64_t b)
> >  
> >  static uint64_t ticks_to_ns(uint64_t value)
> >  {
> > -    return (muldiv64(value, HPET_CLK_PERIOD, FS_PER_NS));
> > +    return value * HPET_CLK_PERIOD;
> >  }
> >  
> >  static uint64_t ns_to_ticks(uint64_t value)
> >  {
> > -    return (muldiv64(value, FS_PER_NS, HPET_CLK_PERIOD));
> > +    return value / HPET_CLK_PERIOD;
> >  }
> >  
> >  static uint64_t hpet_fixup_reg(uint64_t new, uint64_t old, uint64_t mask)
> > @@ -758,7 +758,7 @@ static void hpet_realize(DeviceState *dev, Error
> > **errp)
> >      /* 64-bit main counter; LegacyReplacementRoute. */
> >      s->capability = 0x8086a001ULL;
> >      s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
> > -    s->capability |= ((HPET_CLK_PERIOD) << 32);
> > +    s->capability |= ((uint64_t)(HPET_CLK_PERIOD * FS_PER_NS) << 32);
> >  
> >      qdev_init_gpio_in(dev, hpet_handle_legacy_irq, 2);
> >      qdev_init_gpio_out(dev, &s->pit_enabled, 1);
> > diff --git a/include/hw/timer/hpet.h b/include/hw/timer/hpet.h
> > index 773953b..d872909 100644
> > --- a/include/hw/timer/hpet.h
> > +++ b/include/hw/timer/hpet.h
> > @@ -16,9 +16,9 @@
> >  #include "qom/object.h"
> >  
> >  #define HPET_BASE               0xfed00000
> > -#define HPET_CLK_PERIOD         10000000ULL /* 10000000 femtoseconds ==
> > 10ns*/
> > +#define HPET_CLK_PERIOD         10 /* 10 ns*/
> >  
> > -#define FS_PER_NS 1000000
> > +#define FS_PER_NS 1000000       /* 1000000 femtoseconds == 1 ns */
> >  #define HPET_MIN_TIMERS         3
> >  #define HPET_MAX_TIMERS         32
> >  
> > 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

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

* Re: [Qemu-devel] [PATCH v3 4/9] mips: remove muldiv64()
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 4/9] mips: " Laurent Vivier
  2015-09-08 12:54   ` Laurent Vivier
@ 2015-09-08 13:42   ` Leon Alrae
  1 sibling, 0 replies; 27+ messages in thread
From: Leon Alrae @ 2015-09-08 13:42 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel, Aurelien Jarno

On 27/08/15 20:33, Laurent Vivier wrote:
> Originally, timers were ticks based, and it made sense to
> add ticks to current time to know when to trigger an alarm.
> 
> But since commit:
> 
> 7447545 change all other clock references to use nanosecond resolution accessors
> 
> All timers use nanoseconds and we need to convert ticks to nanoseconds, by
> doing something like:
> 
>     y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)
> 
> where x is the number of device ticks and y the number of system ticks.
> 
> y is used as nanoseconds in timer functions,
> it works because 1 tick is 1 nanosecond.
> (get_ticks_per_sec() is 10^9)
> 
> But as MIPS timer frequency is 100 MHz, we can also do:
> 
>     y = x * 10; /* 100 MHz period is 10 ns */
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/mips/cputimer.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)

Reviewed-by: Leon Alrae <leon.alrae@imgtec.com>

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

* [Qemu-devel] [PATCH v4 1/9] i6300esb: remove muldiv64()
  2015-08-27 19:32 ` [Qemu-devel] [PATCH v3 1/9] i6300esb: remove muldiv64() Laurent Vivier
@ 2015-09-14 13:31   ` Laurent Vivier
  0 siblings, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2015-09-14 13:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, pbonzini, stefanha, mst

Originally, timers were ticks based, and it made sense to
add ticks to current time to know when to trigger an alarm.

But since commit:

7447545 change all other clock references to use nanosecond resolution accessors

All timers use nanoseconds and we need to convert ticks to nanoseconds, by
doing something like:

    y = muldiv64(x, get_ticks_per_sec(), PCI_FREQUENCY)

where x is the number of device ticks and y the number of system ticks.

y is used as nanoseconds in timer functions,
it works because 1 tick is 1 nanosecond.
(get_ticks_per_sec() is 10^9)

But as PCI frequency is 33 MHz, we can also do:

    y = x * 30; /* 33 MHz PCI period is 30 ns */

Which is much more simple.

This implies a 33.333333 MHz PCI frequency,
but this is correct.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v4: only rebase this patch on origin/master as the line
    is modified by commit "fee562e i6300esb: fix timer overflow"

 hw/watchdog/wdt_i6300esb.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index 3e07d44..a91c8fd 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -129,14 +129,9 @@ static void i6300esb_restart_timer(I6300State *d, int stage)
     else
         timeout <<= 5;
 
-    /* Get the timeout in units of ticks_per_sec.
-     *
-     * ticks_per_sec is typically 10^9 == 0x3B9ACA00 (30 bits), with
-     * 20 bits of user supplied preload, and 15 bits of scale, the
-     * multiply here can exceed 64-bits, before we divide by 33MHz, so
-     * we use a higher-precision intermediate result.
-     */
-    timeout = muldiv64(timeout, get_ticks_per_sec(), 33000000);
+    /* Get the timeout in nanoseconds. */
+
+    timeout = timeout * 30; /* on a PCI bus, 1 tick is 30 ns*/
 
     i6300esb_debug("stage %d, timeout %" PRIi64 "\n", d->stage, timeout);
 
-- 
2.1.0

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

* Re: [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64()
  2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
                   ` (8 preceding siblings ...)
  2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 9/9] net: " Laurent Vivier
@ 2015-09-24 16:21 ` Laurent Vivier
  2015-09-25 10:31   ` Paolo Bonzini
  9 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2015-09-24 16:21 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell, Stefan Hajnoczi, Michael S . Tsirkin,
	Paolo Bonzini, Aurelien Jarno, Leon Alrae, Jia Liu, Jason Wang,
	crosthwaitepeter

Hi,

all the patches of the series have been reviewed, except:

  [PATCH v4 1/9] i6300esb: remove muldiv64(),
                  which is a rebased version.
  [PATCH v3 5/9] openrisc: remove muldiv64()

Any volunteers ?

Laurent

On 27/08/2015 21:32, Laurent Vivier wrote:
> Originally, timers were ticks based, and it made sense to
> add ticks to current time to know when to trigger an alarm.
> 
> But since commit:
> 
> 7447545 change all other clock references to use nanosecond resolution accessors
> 
> All timers use nanoseconds and we need to convert ticks to nanoseconds, by
> doing something like:
> 
>      y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)
> 
> where x is the number of device ticks and y the number of system ticks.
> 
> y is used as nanoseconds in timer functions,
> it works because 1 tick is 1 nanosecond.
> (get_ticks_per_sec() is 10^9)
> 
> But if get_ticks_per_sec() / TIMER_FREQ is an integer, we can do:
> 
>     y = x * TIMER_PERIOD;
> 
> v3:
> Split "PCI: remove muldiv64()" in 3 patches:
>     i6300esb: remove muldiv64()
>     rtl8139: remove muldiv64()
>     pcnet: remove muldiv64()
> v2:
> 4/4 For target-arm, don't remove muldiv64() but clarify
>     the use of the values.
> 7/7 Replace qemu_clock_get_ns()/1000 by qemu_clock_get_us()
> 
> Laurent Vivier (9):
>   i6300esb: remove muldiv64()
>   rtl8139: remove muldiv64()
>   pcnet: remove muldiv64()
>   mips: remove muldiv64()
>   openrisc: remove muldiv64()
>   arm: clarify the use of muldiv64()
>   hpet: remove muldiv64()
>   bt: remove muldiv64()
>   net: remove muldiv64()
> 
>  hw/bt/hci.c                |  4 ++--
>  hw/mips/cputimer.c         | 19 ++++++++-----------
>  hw/net/pcnet.c             |  3 +--
>  hw/net/rtl8139.c           | 14 ++++++--------
>  hw/openrisc/cputimer.c     |  7 +++----
>  hw/timer/hpet.c            |  6 +++---
>  hw/watchdog/wdt_i6300esb.c | 11 +++--------
>  include/hw/timer/hpet.h    |  4 ++--
>  net/dump.c                 |  2 +-
>  target-arm/helper.c        | 14 ++++++++------
>  tests/rtl8139-test.c       |  2 +-
>  11 files changed, 38 insertions(+), 48 deletions(-)
> 

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

* Re: [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64()
  2015-09-24 16:21 ` [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
@ 2015-09-25 10:31   ` Paolo Bonzini
  0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2015-09-25 10:31 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel, Peter Maydell, Stefan Hajnoczi,
	Michael S . Tsirkin, Aurelien Jarno, Leon Alrae, Jia Liu,
	Jason Wang, crosthwaitepeter



On 24/09/2015 18:21, Laurent Vivier wrote:
> Hi,
> 
> all the patches of the series have been reviewed, except:
> 
>   [PATCH v4 1/9] i6300esb: remove muldiv64(),
>                   which is a rebased version.
>   [PATCH v3 5/9] openrisc: remove muldiv64()
> 
> Any volunteers ?

They both look good, thanks.  Please send a signed pull request with the
patches!

Paolo

> Laurent
> 
> On 27/08/2015 21:32, Laurent Vivier wrote:
>> Originally, timers were ticks based, and it made sense to
>> add ticks to current time to know when to trigger an alarm.
>>
>> But since commit:
>>
>> 7447545 change all other clock references to use nanosecond resolution accessors
>>
>> All timers use nanoseconds and we need to convert ticks to nanoseconds, by
>> doing something like:
>>
>>      y = muldiv64(x, get_ticks_per_sec(), TIMER_FREQ)
>>
>> where x is the number of device ticks and y the number of system ticks.
>>
>> y is used as nanoseconds in timer functions,
>> it works because 1 tick is 1 nanosecond.
>> (get_ticks_per_sec() is 10^9)
>>
>> But if get_ticks_per_sec() / TIMER_FREQ is an integer, we can do:
>>
>>     y = x * TIMER_PERIOD;
>>
>> v3:
>> Split "PCI: remove muldiv64()" in 3 patches:
>>     i6300esb: remove muldiv64()
>>     rtl8139: remove muldiv64()
>>     pcnet: remove muldiv64()
>> v2:
>> 4/4 For target-arm, don't remove muldiv64() but clarify
>>     the use of the values.
>> 7/7 Replace qemu_clock_get_ns()/1000 by qemu_clock_get_us()
>>
>> Laurent Vivier (9):
>>   i6300esb: remove muldiv64()
>>   rtl8139: remove muldiv64()
>>   pcnet: remove muldiv64()
>>   mips: remove muldiv64()
>>   openrisc: remove muldiv64()
>>   arm: clarify the use of muldiv64()
>>   hpet: remove muldiv64()
>>   bt: remove muldiv64()
>>   net: remove muldiv64()
>>
>>  hw/bt/hci.c                |  4 ++--
>>  hw/mips/cputimer.c         | 19 ++++++++-----------
>>  hw/net/pcnet.c             |  3 +--
>>  hw/net/rtl8139.c           | 14 ++++++--------
>>  hw/openrisc/cputimer.c     |  7 +++----
>>  hw/timer/hpet.c            |  6 +++---
>>  hw/watchdog/wdt_i6300esb.c | 11 +++--------
>>  include/hw/timer/hpet.h    |  4 ++--
>>  net/dump.c                 |  2 +-
>>  target-arm/helper.c        | 14 ++++++++------
>>  tests/rtl8139-test.c       |  2 +-
>>  11 files changed, 38 insertions(+), 48 deletions(-)
>>

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

end of thread, other threads:[~2015-09-25 10:31 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-27 19:32 [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
2015-08-27 19:32 ` [Qemu-devel] [PATCH v3 1/9] i6300esb: remove muldiv64() Laurent Vivier
2015-09-14 13:31   ` [Qemu-devel] [PATCH v4 " Laurent Vivier
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 2/9] rtl8139: " Laurent Vivier
2015-08-28 14:12   ` Stefan Hajnoczi
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 3/9] pcnet: " Laurent Vivier
2015-08-28 14:12   ` Stefan Hajnoczi
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 4/9] mips: " Laurent Vivier
2015-09-08 12:54   ` Laurent Vivier
2015-09-08 13:42   ` Leon Alrae
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 5/9] openrisc: " Laurent Vivier
2015-09-08 12:54   ` Laurent Vivier
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 6/9] arm: clarify the use of muldiv64() Laurent Vivier
2015-08-27 20:23   ` Peter Crosthwaite
2015-09-01 11:17   ` Peter Maydell
2015-09-01 11:23     ` Laurent Vivier
2015-09-01 11:30       ` Peter Maydell
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 7/9] hpet: remove muldiv64() Laurent Vivier
2015-09-08 12:55   ` Laurent Vivier
2015-09-08 12:58     ` Paolo Bonzini
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 8/9] bt: " Laurent Vivier
2015-09-08 12:55   ` Laurent Vivier
2015-09-08 12:57     ` Paolo Bonzini
2015-08-27 19:33 ` [Qemu-devel] [PATCH v3 9/9] net: " Laurent Vivier
2015-08-28 14:12   ` Stefan Hajnoczi
2015-09-24 16:21 ` [Qemu-devel] [PATCH v3 0/9] remove useless muldiv64() Laurent Vivier
2015-09-25 10:31   ` Paolo Bonzini

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.