From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49067) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Um0yc-0004BU-8W for qemu-devel@nongnu.org; Mon, 10 Jun 2013 08:13:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Um0yZ-0007IQ-DQ for qemu-devel@nongnu.org; Mon, 10 Jun 2013 08:13:34 -0400 Message-ID: <51B5C2E7.4040709@suse.de> Date: Mon, 10 Jun 2013 14:13:27 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1370850918-18687-1-git-send-email-Bharat.Bhushan@freescale.com> In-Reply-To: <1370850918-18687-1-git-send-email-Bharat.Bhushan@freescale.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharat Bhushan Cc: scottwood@freescale.com, Bharat Bhushan , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de Am 10.06.2013 09:55, schrieb Bharat Bhushan: > QEMU timer supports a maximum timer of INT64_MAX. So starting timer onl= y for > time which is calculated using target_bit < 62 and deactivate/stop time= r if > the target bit is above 61. >=20 > This patch also fix the time calculation from target_bit. > The code was doing (1 << (target_bit + 1)) while this > should be (1ULL << (target_bit + 1)). >=20 > Signed-off-by: Bharat Bhushan > --- > v1->v2 > - Added "booke: timer:" in patch subject >=20 > hw/ppc/ppc_booke.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) >=20 > diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c > index e41b036..f4eda15 100644 > --- a/hw/ppc/ppc_booke.c > +++ b/hw/ppc/ppc_booke.c > @@ -133,9 +133,15 @@ static void booke_update_fixed_timer(CPUPPCState = *env, > ppc_tb_t *tb_env =3D env->tb_env; > uint64_t lapse; > uint64_t tb; > - uint64_t period =3D 1 << (target_bit + 1); > + uint64_t period; > uint64_t now; > =20 > + /* Deactivate timer for target_bit > 61 */ > + if (target_bit > 61) > + return;=20 Braces missing and trailing whitespace after return. So IIUC we can only allow 63 bits due to signedness, thus a maximum of (1 << 62), thus target_bit <=3D 61. Any chance at least the comment can be worded to explain that any better? Maybe also use (target-bit + 1 >=3D 63) or period > INT64_MAX as condition? Best regards, Andreas > + > + period =3D 1ULL << (target_bit + 1); > + > now =3D qemu_get_clock_ns(vm_clock); > tb =3D cpu_ppc_get_tb(tb_env, now, tb_env->tb_offset); > =20 >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg