Re: [PATCH v1 2/5] timer: arm: Introduce functions to get the host cntfrq

[Andrew Jones]

On Tue, Dec 10, 2019 at 03:47:39PM +0000, Peter Maydell wrote:
> On Wed, 16 Oct 2019 at 15:34, Andrew Jones <address@hidden> wrote:
> >
> > When acceleration like KVM is in use it's necessary to use the host's
> > counter frequency when converting ticks to or from time units.
> >
> > Signed-off-by: Andrew Jones <address@hidden>
> > Reviewed-by: Richard Henderson <address@hidden>
> > ---
> >  include/qemu/timer.h | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> > index 85bc6eb00b21..8941ddea8242 100644
> > --- a/include/qemu/timer.h
> > +++ b/include/qemu/timer.h
> > @@ -1006,6 +1006,22 @@ static inline int64_t cpu_get_host_ticks(void)
> >  }
> >  #endif
> >
> > +#if defined(__aarch64__)
> > +static inline uint32_t cpu_get_host_tick_frequency(void)
> > +{
> > +    uint64_t frq;
> > +    asm volatile("mrs %0, cntfrq_el0" : "=r" (frq));
> > +    return frq;
> > +}
> > +#elif defined(__arm__)
> > +static inline uint32_t cpu_get_host_tick_frequency(void)
> > +{
> > +    uint32_t frq;
> > +    asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (frq));
> > +    return frq;
> > +}
> > +#endif
> 
> Don't we want to know what the guest counter frequency
> is, not the host counter frequency? That is, I would have
> expected that we get this value via doing a KVM ONE_REG ioctl
> to ask the kernel what the guest cntfrq is. Are we using
> the host value on the assumption that the guest might have
> misprogrammed their copy of the register?
>

Indeed it would be best to get whatever KVM is using for the given VCPU's
CNTFRQ through GET_ONE_REG, but KVM doesn't seem to allow it. I hadn't
tested before, but to be sure, I did now with the following kselftests
test

 #include "kvm_util.h"
 #include "processor.h"
 
 static void guest_code(void) { }
 
 int main(void)
 {
   struct kvm_vm *vm = vm_create_default(0, 0, guest_code);
   uint64_t reg;
 
   get_reg(vm, 0, ARM64_SYS_REG(3, 3, 14, 0, 0), &reg);
   printf("%lx\n", reg);
   return 0;
 }

The vcpu ioctl fails with ENOENT. Currently KVM requires all guests to
have the same frequency as the host, but I guess that will change
eventually. It's definitely best to do the save/restore thing, as you
suggest.

Thanks,
drew