[Qemu-devel] [PATCH v2 0/5] muldiv64() trivial fixes

[Qemu-devel] [PATCH v2 0/5] muldiv64() trivial fixes

[Laurent Vivier]

Some fixes in the use of muldiv64()

The patches have been generated with the help of coccinelle.

The first patch contains the scripts used to generate the two following
patches. As it is done for linux, I've added the scripts under
scripts/coccinelle.

v2:
- rework scripts/coccinelle/swap_muldiv64.cocci, to simplify it
- add overflow_muldiv64.cocci and simplify_muldiv64.cocci
- add resulting patches

Laurent Vivier (5):
  scripts: add muldiv64() checking coccinelle scripts
  The only 64bit parameter of muldiv64() is the first one.
  remove useless muldiv64()
  replace muldiv64(a, b, c) by (uint64_t)a * b / c
  ppc: Remove a potential overflow in muldiv64()

 hw/audio/gus.c                             |  2 +-
 hw/ppc/ppc.c                               |  2 +-
 hw/timer/omap_gptimer.c                    |  4 ++--
 hw/usb/hcd-ohci.c                          |  2 +-
 hw/xtensa/pic_cpu.c                        |  4 ++--
 scripts/coccinelle/overflow_muldiv64.cocci | 16 ++++++++++++++++
 scripts/coccinelle/remove_muldiv64.cocci   |  6 ++++++
 scripts/coccinelle/simplify_muldiv64.cocci | 11 +++++++++++
 scripts/coccinelle/swap_muldiv64.cocci     | 13 +++++++++++++
 9 files changed, 53 insertions(+), 7 deletions(-)
 create mode 100644 scripts/coccinelle/overflow_muldiv64.cocci
 create mode 100644 scripts/coccinelle/remove_muldiv64.cocci
 create mode 100644 scripts/coccinelle/simplify_muldiv64.cocci
 create mode 100644 scripts/coccinelle/swap_muldiv64.cocci

-- 
2.5.5

[Qemu-devel] [PATCH v2 1/5] scripts: add muldiv64() checking coccinelle scripts

[Laurent Vivier]

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 scripts/coccinelle/overflow_muldiv64.cocci | 16 ++++++++++++++++
 scripts/coccinelle/remove_muldiv64.cocci   |  6 ++++++
 scripts/coccinelle/simplify_muldiv64.cocci | 11 +++++++++++
 scripts/coccinelle/swap_muldiv64.cocci     | 13 +++++++++++++
 4 files changed, 46 insertions(+)
 create mode 100644 scripts/coccinelle/overflow_muldiv64.cocci
 create mode 100644 scripts/coccinelle/remove_muldiv64.cocci
 create mode 100644 scripts/coccinelle/simplify_muldiv64.cocci
 create mode 100644 scripts/coccinelle/swap_muldiv64.cocci

diff --git a/scripts/coccinelle/overflow_muldiv64.cocci b/scripts/coccinelle/overflow_muldiv64.cocci
new file mode 100644
index 0000000..08ec4a8
--- /dev/null
+++ b/scripts/coccinelle/overflow_muldiv64.cocci
@@ -0,0 +1,16 @@
+// Find muldiv64(i64, i64, x) for potential overflow
+@filter@
+typedef uint64_t;
+typedef int64_t;
+{ uint64_t, int64_t, long, unsigned long } a, b;
+expression c;
+position p;
+@@
+
+muldiv64(a,b,c)@p
+
+@script:python@
+p << filter.p;
+@@
+
+cocci.print_main("potential muldiv64() overflow", p)
diff --git a/scripts/coccinelle/remove_muldiv64.cocci b/scripts/coccinelle/remove_muldiv64.cocci
new file mode 100644
index 0000000..4c10bd5
--- /dev/null
+++ b/scripts/coccinelle/remove_muldiv64.cocci
@@ -0,0 +1,6 @@
+// replace muldiv64(a, 1, b) by "a / b"
+@@
+expression a, b;
+@@
+-muldiv64(a, 1, b)
++a / b
diff --git a/scripts/coccinelle/simplify_muldiv64.cocci b/scripts/coccinelle/simplify_muldiv64.cocci
new file mode 100644
index 0000000..3d7c974
--- /dev/null
+++ b/scripts/coccinelle/simplify_muldiv64.cocci
@@ -0,0 +1,11 @@
+// replace muldiv64(i32, i32, x) by (uint64_t)i32 * i32 / x
+@@
+typedef uint32_t;
+typedef int32_t;
+{ uint32_t, int32_t, int, unsigned int } a, b;
+typedef uint64_t;
+expression c;
+@@
+
+-muldiv64(a,b,c)
++(uint64_t) a * b / c
diff --git a/scripts/coccinelle/swap_muldiv64.cocci b/scripts/coccinelle/swap_muldiv64.cocci
new file mode 100644
index 0000000..b48b0d0
--- /dev/null
+++ b/scripts/coccinelle/swap_muldiv64.cocci
@@ -0,0 +1,13 @@
+// replace muldiv64(i32, i64, x) by muldiv64(i64, i32, x)
+@@
+typedef uint64_t;
+typedef int64_t;
+typedef uint32_t;
+typedef int32_t;
+{ uint32_t, int32_t, int, unsigned int } a;
+{ uint64_t, int64_t, long, unsigned long } b;
+expression c;
+@@
+
+-muldiv64(a,b,c)
++muldiv64(b,a,c)
-- 
2.5.5

[Qemu-devel] [PATCH v2 2/5] The only 64bit parameter of muldiv64() is the first one.

[Laurent Vivier]

muldiv64() is "uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)"

Some time it is used as muldiv64(uint32_t a, uint64_t b, uint32_t c)"

This patch is the result of coccinelle script
scripts/coccinelle/swap_muldiv64.cocci to reorder arguments.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 hw/timer/omap_gptimer.c | 4 ++--
 hw/usb/hcd-ohci.c       | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/timer/omap_gptimer.c b/hw/timer/omap_gptimer.c
index 3a43863..5e3e8a6 100644
--- a/hw/timer/omap_gptimer.c
+++ b/hw/timer/omap_gptimer.c
@@ -133,8 +133,8 @@ static inline void omap_gp_timer_update(struct omap_gp_timer_s *timer)
         timer_mod(timer->timer, timer->time + expires);
 
         if (timer->ce && timer->match_val >= timer->val) {
-            matches = muldiv64(timer->match_val - timer->val,
-                            timer->ticks_per_sec, timer->rate);
+            matches = muldiv64(timer->ticks_per_sec,
+                               timer->match_val - timer->val, timer->rate);
             timer_mod(timer->match, timer->time + matches);
         } else
             timer_del(timer->match);
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index ffab561..e5b535f 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1474,7 +1474,7 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
     if (tks >= usb_frame_time)
         return (ohci->frt << 31);
 
-    tks = muldiv64(1, tks, usb_bit_time);
+    tks = muldiv64(tks, 1, usb_bit_time);
     fr = (uint16_t)(ohci->fi - tks);
 
     return (ohci->frt << 31) | fr;
-- 
2.5.5

[Qemu-devel] [PATCH v2 3/5] remove useless muldiv64()

[Laurent Vivier]

muldiv64(a, 1, b) is like "a / b".

This patch is the result of coccinelle script
scripts/coccinelle/remove_muldiv64.cocci.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 hw/usb/hcd-ohci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index e5b535f..2af6c16 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1474,7 +1474,7 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
     if (tks >= usb_frame_time)
         return (ohci->frt << 31);
 
-    tks = muldiv64(tks, 1, usb_bit_time);
+    tks = tks / usb_bit_time;
     fr = (uint16_t)(ohci->fi - tks);
 
     return (ohci->frt << 31) | fr;
-- 
2.5.5

[Qemu-devel] [PATCH v2 4/5] replace muldiv64(a, b, c) by (uint64_t)a * b / c

[Laurent Vivier]

When "a" and "b" are 32bit values, we don't have to cast
them to 128bit, 64bit is enough.

This patch is the result of coccinelle script
scripts/coccinelle/simplify_muldiv64.cocci

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/audio/gus.c      | 2 +-
 hw/xtensa/pic_cpu.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index 9dd6947..6c02646 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -144,7 +144,7 @@ static void GUS_callback (void *opaque, int free)
     s->left = samples;
 
  reset:
-    gus_irqgen (&s->emu, muldiv64 (net, 1000000, s->freq));
+    gus_irqgen (&s->emu, (uint64_t)net * 1000000 / s->freq);
 }
 
 int GUS_irqrequest (GUSEmuState *emu, int hwirq, int n)
diff --git a/hw/xtensa/pic_cpu.c b/hw/xtensa/pic_cpu.c
index c835bd0..343ea2c 100644
--- a/hw/xtensa/pic_cpu.c
+++ b/hw/xtensa/pic_cpu.c
@@ -121,8 +121,8 @@ void xtensa_rearm_ccompare_timer(CPUXtensaState *env)
     }
     env->wake_ccount = wake_ccount;
     timer_mod(env->ccompare_timer, env->halt_clock +
-            muldiv64(wake_ccount - env->sregs[CCOUNT],
-                1000000, env->config->clock_freq_khz));
+            (uint64_t)(wake_ccount - env->sregs[CCOUNT]) *
+            1000000 / env->config->clock_freq_khz);
 }
 
 static void xtensa_ccompare_cb(void *opaque)
-- 
2.5.5

[Qemu-devel] [PATCH v2 5/5] ppc: Remove a potential overflow in muldiv64()

[Laurent Vivier]

The coccinelle script:
scripts/coccinelle/overflow_muldiv64.cocci
gives us a list of potential overflows in muldiv64()
(the two first parameters are 64bit values).

This patch fixes one, as the fix seems obvious:

replace muldiv64(a, b, c) by muldiv64(b, a, c)
as "a" and "b" are 64bit values but a <= NANOSECONDS_PER_SECOND.
(10^9 -> 30bit value).

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/ppc/ppc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 38ff2e1..07ea47c 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -880,7 +880,7 @@ static int timebase_post_load(void *opaque, int version_id)
     host_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST);
     ns_diff = MAX(0, host_ns - tb_remote->time_of_the_day_ns);
     migration_duration_ns = MIN(NANOSECONDS_PER_SECOND, ns_diff);
-    migration_duration_tb = muldiv64(migration_duration_ns, freq,
+    migration_duration_tb = muldiv64(freq, migration_duration_ns,
                                      NANOSECONDS_PER_SECOND);
     guest_tb = tb_remote->guest_timebase + MIN(0, migration_duration_tb);
 
-- 
2.5.5

Re: [Qemu-devel] [PATCH v2 4/5] replace muldiv64(a, b, c) by (uint64_t)a * b / c

[Max Filippov]

On Mon, May 9, 2016 at 4:24 PM, Laurent Vivier <lvivier@redhat.com> wrote:
> When "a" and "b" are 32bit values, we don't have to cast
> them to 128bit, 64bit is enough.
>
> This patch is the result of coccinelle script
> scripts/coccinelle/simplify_muldiv64.cocci
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/audio/gus.c      | 2 +-
>  hw/xtensa/pic_cpu.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

For xtensa PIC:
Acked-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH v2 0/5] muldiv64() trivial fixes

[Laurent Vivier]

Gerd, I forgot to CC: you for the USB, audio patches,
Peter, for the OMAP one,
Alex, for the PPC one,
[Max Filippov has already acked the xtensa one, thanks!]

Thanks,
Laurent

On 09/05/2016 15:24, Laurent Vivier wrote:
> Some fixes in the use of muldiv64()
> 
> The patches have been generated with the help of coccinelle.
> 
> The first patch contains the scripts used to generate the two following
> patches. As it is done for linux, I've added the scripts under
> scripts/coccinelle.
> 
> v2:
> - rework scripts/coccinelle/swap_muldiv64.cocci, to simplify it
> - add overflow_muldiv64.cocci and simplify_muldiv64.cocci
> - add resulting patches
> 
> Laurent Vivier (5):
>   scripts: add muldiv64() checking coccinelle scripts
>   The only 64bit parameter of muldiv64() is the first one.
>   remove useless muldiv64()
>   replace muldiv64(a, b, c) by (uint64_t)a * b / c
>   ppc: Remove a potential overflow in muldiv64()
> 
>  hw/audio/gus.c                             |  2 +-
>  hw/ppc/ppc.c                               |  2 +-
>  hw/timer/omap_gptimer.c                    |  4 ++--
>  hw/usb/hcd-ohci.c                          |  2 +-
>  hw/xtensa/pic_cpu.c                        |  4 ++--
>  scripts/coccinelle/overflow_muldiv64.cocci | 16 ++++++++++++++++
>  scripts/coccinelle/remove_muldiv64.cocci   |  6 ++++++
>  scripts/coccinelle/simplify_muldiv64.cocci | 11 +++++++++++
>  scripts/coccinelle/swap_muldiv64.cocci     | 13 +++++++++++++
>  9 files changed, 53 insertions(+), 7 deletions(-)
>  create mode 100644 scripts/coccinelle/overflow_muldiv64.cocci
>  create mode 100644 scripts/coccinelle/remove_muldiv64.cocci
>  create mode 100644 scripts/coccinelle/simplify_muldiv64.cocci
>  create mode 100644 scripts/coccinelle/swap_muldiv64.cocci
> 

Re: [Qemu-devel] [PATCH v2 0/5] muldiv64() trivial fixes

[Michael Tokarev]

09.05.2016 16:24, Laurent Vivier wrote:
> Some fixes in the use of muldiv64()
> 
> The patches have been generated with the help of coccinelle.
> 
> The first patch contains the scripts used to generate the two following
> patches. As it is done for linux, I've added the scripts under
> scripts/coccinelle.
> 
> v2:
> - rework scripts/coccinelle/swap_muldiv64.cocci, to simplify it
> - add overflow_muldiv64.cocci and simplify_muldiv64.cocci
> - add resulting patches
> 
> Laurent Vivier (5):
>   scripts: add muldiv64() checking coccinelle scripts
>   The only 64bit parameter of muldiv64() is the first one.
>   remove useless muldiv64()
>   replace muldiv64(a, b, c) by (uint64_t)a * b / c
>   ppc: Remove a potential overflow in muldiv64()

Applied series to -trivial, thanks!

/mjt