From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ycx0g-0005uM-Ny for qemu-devel@nongnu.org; Tue, 31 Mar 2015 10:19:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ycx0c-0000qU-L1 for qemu-devel@nongnu.org; Tue, 31 Mar 2015 10:19:18 -0400 Received: from mail-wg0-x235.google.com ([2a00:1450:400c:c00::235]:34920) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ycx0c-0000qI-7y for qemu-devel@nongnu.org; Tue, 31 Mar 2015 10:19:14 -0400 Received: by wgdm6 with SMTP id m6so20962929wgd.2 for ; Tue, 31 Mar 2015 07:19:13 -0700 (PDT) Date: Tue, 31 Mar 2015 15:19:11 +0100 From: Stefan Hajnoczi Message-ID: <20150331141911.GD27156@stefanha-thinkpad.redhat.com> References: <1427559731-20180-1-git-send-email-emilcondrea@gmail.com> <20150330130109.GI25181@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="C+ts3FVlLX8+P6JN" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] replaced get_ticks_per_sec() with constant List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , Emil Condrea , QEMU Developers , Stefan Hajnoczi , Alistair Francis --C+ts3FVlLX8+P6JN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 31, 2015 at 09:46:32AM +1000, Peter Crosthwaite wrote: > On Tue, Mar 31, 2015 at 7:52 AM, Peter Maydell = wrote: > > On 30 March 2015 at 14:01, Stefan Hajnoczi wrote: > >> On Sat, Mar 28, 2015 at 06:22:11PM +0200, Emil Condrea wrote: > >>> diff --git a/target-arm/helper.c b/target-arm/helper.c > >>> index 10886c5..504530a 100644 > >>> --- a/target-arm/helper.c > >>> +++ b/target-arm/helper.c > >>> @@ -649,7 +649,7 @@ void pmccntr_sync(CPUARMState *env) > >>> uint64_t temp_ticks; > >>> > >>> temp_ticks =3D muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL), > >>> - get_ticks_per_sec(), 1000000); > >>> + NSEC_PER_SEC, 1000000); > >>> > >>> if (env->cp15.c9_pmcr & PMCRD) { > >>> /* Increment once every 64 processor clock cycles */ > >>> @@ -688,7 +688,7 @@ static uint64_t pmccntr_read(CPUARMState *env, co= nst ARMCPRegInfo *ri) > >>> } > >>> > >>> total_ticks =3D muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL), > >>> - get_ticks_per_sec(), 1000000); > >>> + NSEC_PER_SEC, 1000000); > >>> > >>> if (env->cp15.c9_pmcr & PMCRD) { > >>> /* Increment once every 64 processor clock cycles */ > >>> @@ -709,7 +709,7 @@ static void pmccntr_write(CPUARMState *env, const= ARMCPRegInfo *ri, > >>> } > >>> > >>> total_ticks =3D muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL), > >>> - get_ticks_per_sec(), 1000000); > >>> + NSEC_PER_SEC, 1000000); > >> > >> Peter: I wonder why the muldiv64() expression is necessary. Is it > >> intentionally returning the microsecond clock converted to nanoseconds? > >> >=20 > That is the effect but not the intention. In this context, > get_tick_per_sec() is really trying to be a clock signal frequency > rate and QEMU ARM is just hardcoding to 1GHz. Because its fixed and in > terms of real time, your proposed cancellation is possible. >=20 > For ARM CPU we are simplifying the CPU clock pin as ns (basically 1GHz > clock). Ideally this should be parameterisable as this clock is > controlled outside of the ARM processor. >=20 > I think the form of the current code has value for self documentation, > and maybe should have a comment explaining this 1GHz harcodedness. > When we get around to parameterising the ARM CPU frequency as a QOM > property this code should be fixed by doing: >=20 > s/NSEC_PER_SEC/s->clock_freq_hz/ >=20 > So this muldiv in its current form is really preparation for that. Can > we keep it? Your patch as-is looks good to me. >=20 > Acked-by: Peter Crosthwaite Thanks for explaining. Stefan --C+ts3FVlLX8+P6JN Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVGqzfAAoJEJykq7OBq3PIwFEIAKKi98jGkodvAbME1CIb5FOa Hmn8W48HF0q4+mMNRXfAdv+8kXHn9/RXgbigA2RToEW89QregOKAHI9BGkOlQPUO Xln2TJM8dCr9StEb8f2bYe+26bq71OgmXbVlGEZdCbX61xVIWWRMxJXXFgqTPU+V DS3azwpSq/6iz9adQqJZP1Z5ioSgld01dzRonqEPHvC9PvDAW3DqWsUteyQcANDv B+JP0Lk1euGs7s2F1YF6w0Y+39wqLHtKY6XqnVqef+6c5vNY9wbyiDv5IL4n4YAN Jy6NB5cMPT/1mm9NWT/dy+azYLwUlShG/ORqLR41opG/dptrYJjoI2zPp1LFnGk= =1XVO -----END PGP SIGNATURE----- --C+ts3FVlLX8+P6JN--