All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Laurent Vivier <lvivier@redhat.com>
Cc: qemu-trivial@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, "Richard W.M. Jones" <rjones@redhat.com>
Subject: Re: [Qemu-trivial] [PATCH][TRIVIAL] i6300esb: fix timer overflow
Date: Wed, 5 Aug 2015 10:01:02 +1000	[thread overview]
Message-ID: <20150805000102.GB7739@voom.redhat.com> (raw)
In-Reply-To: <1438676851-10684-1-git-send-email-lvivier@redhat.com>

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

On Tue, Aug 04, 2015 at 10:27:31AM +0200, Laurent Vivier wrote:
> We use muldiv64() to compute the time to wait:
> 
>     timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000);
> 
> but get_ticks_per_sec() is 10^9 (30 bit value) and timeout
> is a 35 bit value.
> 
> Whereas muldiv64 is:
> 
>     uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> 
> So we loose 3 bits of timeout.
> 
> Swapping get_ticks_per_sec() and timeout fixes it.
> 
> We can also replace it by a multiplication by 30 ns,
> but this changes PCI clock frequency from 33MHz to 33.333333MHz
> and we need to do this on all the QEMU PCI devices (later...)
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Hah.  32-bit second argument.  Totally missed that when I put the
muldiv64() in there.  So I didn't eliminate the overflow, just pushed
it out some bits.

Thanks for finding this.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: David Gibson <david@gibson.dropbear.id.au>
To: Laurent Vivier <lvivier@redhat.com>
Cc: qemu-trivial@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, "Richard W.M. Jones" <rjones@redhat.com>
Subject: Re: [Qemu-devel] [PATCH][TRIVIAL] i6300esb: fix timer overflow
Date: Wed, 5 Aug 2015 10:01:02 +1000	[thread overview]
Message-ID: <20150805000102.GB7739@voom.redhat.com> (raw)
In-Reply-To: <1438676851-10684-1-git-send-email-lvivier@redhat.com>

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

On Tue, Aug 04, 2015 at 10:27:31AM +0200, Laurent Vivier wrote:
> We use muldiv64() to compute the time to wait:
> 
>     timeout = muldiv64(get_ticks_per_sec(), timeout, 33000000);
> 
> but get_ticks_per_sec() is 10^9 (30 bit value) and timeout
> is a 35 bit value.
> 
> Whereas muldiv64 is:
> 
>     uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> 
> So we loose 3 bits of timeout.
> 
> Swapping get_ticks_per_sec() and timeout fixes it.
> 
> We can also replace it by a multiplication by 30 ns,
> but this changes PCI clock frequency from 33MHz to 33.333333MHz
> and we need to do this on all the QEMU PCI devices (later...)
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Hah.  32-bit second argument.  Totally missed that when I put the
muldiv64() in there.  So I didn't eliminate the overflow, just pushed
it out some bits.

Thanks for finding this.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2015-08-05  0:01 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-03 13:52 [Qemu-devel] [PATCH] i6300esb: correctly convert watchdog clock ticks into nanoseconds Laurent Vivier
2015-08-03 14:28 ` Laurent Vivier
2015-08-03 14:46   ` Richard W.M. Jones
2015-08-03 15:02     ` Paolo Bonzini
2015-08-03 15:13       ` Laurent Vivier
2015-08-03 15:18         ` Paolo Bonzini
2015-08-03 15:35           ` Laurent Vivier
2015-08-03 16:06             ` Paolo Bonzini
2015-08-03 15:06     ` Laurent Vivier
2015-08-04  8:27 ` [Qemu-trivial] [PATCH][TRIVIAL] i6300esb: fix timer overflow Laurent Vivier
2015-08-04  8:27   ` [Qemu-devel] " Laurent Vivier
2015-08-04 13:47   ` [Qemu-trivial] " Richard W.M. Jones
2015-08-04 13:47     ` [Qemu-devel] " Richard W.M. Jones
2015-08-05  0:01   ` David Gibson [this message]
2015-08-05  0:01     ` David Gibson
2015-09-06 10:29   ` [Qemu-trivial] " Michael Tokarev
2015-09-06 10:29     ` [Qemu-devel] " Michael Tokarev
2015-09-06 14:35     ` [Qemu-trivial] " Paolo Bonzini
2015-09-06 14:35       ` [Qemu-devel] " Paolo Bonzini
2015-09-06 14:41       ` [Qemu-trivial] " Laurent Vivier
2015-09-06 14:41         ` Laurent Vivier
2015-08-04 10:13 ` [Qemu-devel] [PATCH] i6300esb: correctly convert watchdog clock ticks into nanoseconds Richard W.M. Jones
2015-08-04 10:25   ` Laurent Vivier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150805000102.GB7739@voom.redhat.com \
    --to=david@gibson.dropbear.id.au \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=rjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.