On Tue, Mar 31, 2015 at 09:46:32AM +1000, Peter Crosthwaite wrote:
> On Tue, Mar 31, 2015 at 7:52 AM, Peter Maydell <peter.mayd...@linaro.org>
> wrote:
> > On 30 March 2015 at 14:01, Stefan Hajnoczi <stefa...@redhat.com> 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 = 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, const
> >>> ARMCPRegInfo *ri)
> >>> }
> >>>
> >>> total_ticks = 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 = 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?
> >>
>
> 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.
>
> 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.
>
> 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:
>
> s/NSEC_PER_SEC/s->clock_freq_hz/
>
> So this muldiv in its current form is really preparation for that. Can
> we keep it? Your patch as-is looks good to me.
>
> Acked-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>Thanks for explaining. Stefan
pgpIM02ClCUhu.pgp
Description: PGP signature
