Re: [Qemu-devel] [RFC 5/5] arm: Simplify cycle counter

[Peter Crosthwaite]

On Thu, Apr 30, 2015 at 11:14 AM, Christopher Covington
<address@hidden> wrote:
> Present a system with an instructions per cycle of exactly one.
> This makes it less likely a user will mistake the cycle counter
> values as meaningful and makes calculations involving cycles
> trivial while preserving the necessary property of the cycle
> counter register as monotonically increasing.
>
> Signed-off-by: Christopher Covington <address@hidden>
> ---
>  target-arm/helper.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 3e6fb0b..a027a19 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -648,8 +648,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);
> +    temp_ticks = cpu_get_icount_raw();

So I have guests (for better or worse) that make assumptions about the
rate of the PMCCNTR WRT real time. But isn't the PMCCNTR really a
clock cycle counter rather than an instruction counter? That clock
rate is well defined even if it is just the trivial
get_ticks_per_sec() at the moment. Ideally we should have a
configurable clock rate in there instead of get_ticks_per_sec(). This
is a major change in definition.

I can see your use case though, where you actually want this to mean
something WRT program performance. Should we add a switch between the
two behaviours?

Regards,
Peter

>
>      if (env->cp15.c9_pmcr & PMCRD) {
>          /* Increment once every 64 processor clock cycles */
> @@ -687,8 +686,7 @@ static uint64_t pmccntr_read(CPUARMState *env, const 
> ARMCPRegInfo *ri)
>          return env->cp15.c15_ccnt;
>      }
>
> -    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
> -                           get_ticks_per_sec(), 1000000);
> +    total_ticks = cpu_get_icount_raw();
>
>      if (env->cp15.c9_pmcr & PMCRD) {
>          /* Increment once every 64 processor clock cycles */
> @@ -708,8 +706,7 @@ static void pmccntr_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
>          return;
>      }
>
> -    total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
> -                           get_ticks_per_sec(), 1000000);
> +    total_ticks = cpu_get_icount_raw();
>
>      if (env->cp15.c9_pmcr & PMCRD) {
>          /* Increment once every 64 processor clock cycles */
> --
> 1.9.1
>
>