On Mon, Aug 03, 2015 at 04:28:14PM +0200, Laurent Vivier wrote:
>
>
> On 03/08/2015 15:52, Laurent Vivier wrote:
> > Originally, qemu_mod_timer() was using ticks to count time.
> > And i6300esb was converting internal clock ticks (33 MHz) to
> > QEMU timer ticks.
> >
> > The timer has been changed by a script to use nanoseconds:
> >
> > 7447545 change all other clock references to use
> > nanosecond resolution accessors
> >
> > As i6300esb takes nanoseconds, we don't need anymore to
> > multiply counter by get_ticks_per_sec()/33MHz, but instead
> > we must convert watchdog ticks into nanoseconds.
> >
> > Signed-off-by: Laurent Vivier <lviv...@redhat.com>
> > ---
> > hw/watchdog/wdt_i6300esb.c | 25 +++++++++++++++----------
> > 1 file changed, 15 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
> > index cfa2b1b..21119ab 100644
> > --- a/hw/watchdog/wdt_i6300esb.c
> > +++ b/hw/watchdog/wdt_i6300esb.c
> > @@ -124,19 +124,24 @@ static void i6300esb_restart_timer(I6300State *d, int
> > stage)
> > else
> > timeout = d->timer2_preload;
> >
> > - if (d->clock_scale == CLOCK_SCALE_1KHZ)
> > + /* convert timeout to 33Mhz clock ticks */
> > + if (d->clock_scale == CLOCK_SCALE_1KHZ) {
> > + /* The 20-bit Preload Value is loaded into bits 34:15 of the
> > + * main down counter. [...] The approximate clock generated
> > + * is 1 KHz, (Default)
> > + */
> > timeout <<= 15;
> > - else
> > + } else {
> > + /* The 20-bit Preload Value is loaded into bits 24:5 of the
> > + * main down counter. [...] The approximate clock generated
> > + * is 1 MHz.
> > + */
> > 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.
> > + }
> > + /* A 33 Mhz clock gives a 30 ns tick,
> > + * convert timeout from ticks to ns
> > */
> > - timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000);
> > + timeout *= 30;
>
> I'm wondering if a 33 Mhz clock is 33000000 Hz or 33333333 Hz ?
>From the datasheet (chapter 16):
https://www-ssl.intel.com/content/www/us/en/chipsets/6300esb-io-controller-hub-datasheet.html
it says "33 MHz clock (30 ns clock ticks)" which is contradictory.
I suspect it does really mean 33,333,333 Mz (== 30 ns ticks) since
that is the base clock speed discussed elsewhere in that document.
> if this is the former, I should use "timeout = timeout * 1000 / 33" instead.
> (35 bit value * 10 bit value = 45 bit value, it fits in a 64 bit integer)
See also commit 4bc7b4d56657ebf75b986ad46e959cf7232ff26a and the
discussion here:
https://lists.gnu.org/archive/html/qemu-ppc/2015-03/threads.html#00448
The original code did `get_ticks_per_sec() * timeout / 33000000' which
definitely overflowed.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
