[Qemu-devel] [PATCH v2 0/7] remove useless muldiv64()

[Qemu-devel] [PATCH v2 0/7] remove useless muldiv64()

[Laurent Vivier]

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;

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 (7):
  PCI: 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

[Qemu-devel] [PATCH v2 1/7] PCI: remove muldiv64()

[Laurent Vivier]

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>
---
 hw/net/pcnet.c             |  3 +--
 hw/net/rtl8139.c           | 14 ++++++--------
 hw/watchdog/wdt_i6300esb.c | 11 +++--------
 tests/rtl8139-test.c       |  2 +-
 4 files changed, 11 insertions(+), 19 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;
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/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);
 
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

Re: [Qemu-devel] [PATCH v2 1/7] PCI: remove muldiv64()

[Michael S. Tsirkin]

On Wed, Aug 26, 2015 at 08:05:32PM +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>

I think you should split this with a patch per devices.
Otherwise, it's up to net maintainers: pls Cc them all.
And I don't think qemu-trivial is appropriate here.

> ---
>  hw/net/pcnet.c             |  3 +--
>  hw/net/rtl8139.c           | 14 ++++++--------
>  hw/watchdog/wdt_i6300esb.c | 11 +++--------
>  tests/rtl8139-test.c       |  2 +-
>  4 files changed, 11 insertions(+), 19 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;
> 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) ) )

Pls rename this RTL8139_PCI_PERIOD to avoid namespace pollution.

> @@ -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/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);
>  
> 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

[Qemu-devel] [PATCH v2 2/7] mips: remove muldiv64()

[Laurent Vivier]

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

[Qemu-devel] [PATCH v2 3/7] openrisc: remove muldiv64()

[Laurent Vivier]

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

[Qemu-devel] [PATCH v2 4/7] arm: clarify the use of muldiv64()

[Laurent Vivier]

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

[Qemu-devel] [PATCH v2 5/7] hpet: remove muldiv64()

[Laurent Vivier]

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

[Qemu-devel] [PATCH v2 6/7] bt: remove muldiv64()

[Laurent Vivier]

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

[Qemu-devel] [PATCH v2 7/7] net: remove muldiv64()

[Laurent Vivier]

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