From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Um337-0000hR-HX for qemu-devel@nongnu.org; Mon, 10 Jun 2013 10:26:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Um336-0001OP-0z for qemu-devel@nongnu.org; Mon, 10 Jun 2013 10:26:21 -0400 Message-ID: <51B5E20A.8030906@suse.de> Date: Mon, 10 Jun 2013 16:26:18 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1370850918-18687-1-git-send-email-Bharat.Bhushan@freescale.com> <51B5C2E7.4040709@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D070656C8@039-SN2MPN1-012.039d.mgd.msft.net> In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D070656C8@039-SN2MPN1-012.039d.mgd.msft.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: Bhushan Bharat-R65777 Cc: Wood Scott-B07421 , "qemu-ppc@nongnu.org" , =?ISO-8859-1?Q?Andreas_F=E4rber?= , "qemu-devel@nongnu.org" On 06/10/2013 02:47 PM, Bhushan Bharat-R65777 wrote: > >> -----Original Message----- >> From: Andreas F=E4rber [mailto:afaerber@suse.de] >> Sent: Monday, June 10, 2013 5:43 PM >> To: Bhushan Bharat-R65777 >> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; agraf@suse.de; Wood Sc= ott- >> B07421; Bhushan Bharat-R65777 >> Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for >> target_bit above 61 >> >> Am 10.06.2013 09:55, schrieb Bharat Bhushan: >>> QEMU timer supports a maximum timer of INT64_MAX. So starting timer >>> only for time which is calculated using target_bit< 62 and >>> deactivate/stop timer if the target bit is above 61. >>> >>> 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)). >>> >>> Signed-off-by: Bharat Bhushan >>> --- >>> v1->v2 >>> - Added "booke: timer:" in patch subject >>> >>> hw/ppc/ppc_booke.c | 8 +++++++- >>> 1 files changed, 7 insertions(+), 1 deletions(-) >>> >>> 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; >>> >>> + /* Deactivate timer for target_bit> 61 */ >>> + if (target_bit> 61) >>> + return; >> Braces missing and trailing whitespace after return. > Ok, will correct > >> 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 bett= er? Maybe >> also use (target-bit + 1>=3D 63) or period> INT64_MAX as condition? > How about this: > /* QEMU timer supports a maximum timer of INT64_MAX (0x7fffffff_ff= ffffff). > * Run booke fit/wdog timer when > * ((1ULL<< target_bit + 1)< 0x40000000_00000000), i.e target_bi= t =3D 61. > * Also the time with this maximum target_bit (with current range = of > * CPU frequency PowerPC supports) will be many many years. So it = is > * pretty safe to stop the timer above this threshold. */ How about /* This timeout will take years to trigger. Treat the timer as=20 disabled. */ Alex