All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Jones <drjones@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Marc Zyngier <maz@kernel.org>, qemu-arm <qemu-arm@nongnu.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	bijan.mottahedeh@oracle.com
Subject: Re: [PATCH v1 2/5] timer: arm: Introduce functions to get the host cntfrq
Date: Tue, 10 Dec 2019 17:41:08 +0100	[thread overview]
Message-ID: <20191210164108.waqpn3wbo75nwqpx@kamzik.brq.redhat.com> (raw)
In-Reply-To: <CAFEAcA-a9O88EbKqSNxb_7GLVZzC+dx0daEWyAAe9SS4SRa1oQ@mail.gmail.com>

On Tue, Dec 10, 2019 at 03:47:39PM +0000, Peter Maydell wrote:
> On Wed, 16 Oct 2019 at 15:34, Andrew Jones <drjones@redhat.com> 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 <drjones@redhat.com>
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  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



  reply	other threads:[~2019-12-10 16:42 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-16 14:34 [PATCH v1 0/5] target/arm/kvm: Provide an option to adjust virtual time Andrew Jones
2019-10-16 14:34 ` [PATCH v1 1/5] target/arm/kvm64: kvm64 cpus have timer registers Andrew Jones
2019-10-16 14:34 ` [PATCH v1 2/5] timer: arm: Introduce functions to get the host cntfrq Andrew Jones
2019-12-10 15:47   ` Peter Maydell
2019-12-10 16:41     ` Andrew Jones [this message]
2019-10-16 14:34 ` [PATCH v1 3/5] target/arm/kvm: Implement cpu feature kvm-adjvtime Andrew Jones
2019-12-10 15:54   ` Peter Maydell
2019-12-10 16:10     ` Andrew Jones
2019-10-16 14:34 ` [PATCH v1 4/5] tests/arm-cpu-features: Check feature default values Andrew Jones
2019-10-16 14:34 ` [PATCH v1 5/5] target/arm/cpu: Add the kvm-adjvtime CPU property Andrew Jones
2019-10-17 21:17 ` [PATCH v1 0/5] target/arm/kvm: Provide an option to adjust virtual time Masayoshi Mizuma
2019-10-18 12:02   ` Andrew Jones
2019-10-28 18:39     ` Masayoshi Mizuma
2019-12-06 15:22 ` Peter Maydell
2019-12-06 15:53   ` Andrew Jones
2019-12-10  9:51     ` Andrea Bolognani
2019-12-10 10:29       ` Peter Maydell
2019-12-10 11:05         ` Andrew Jones
2019-12-10 11:48           ` Peter Maydell
2019-12-10 13:32             ` Andrew Jones
2019-12-10 14:21               ` Andrea Bolognani
2019-12-10 14:33                 ` Andrew Jones
2019-12-10 15:47                   ` Andrea Bolognani
2019-12-10 16:08                     ` Andrew Jones
2019-12-10 17:16                       ` Andrea Bolognani
2019-12-10 15:45               ` Peter Maydell
2019-12-11  8:02   ` Guoheyi
2019-12-11  9:00     ` Andrew Jones
2019-12-11 13:50       ` Guoheyi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191210164108.waqpn3wbo75nwqpx@kamzik.brq.redhat.com \
    --to=drjones@redhat.com \
    --cc=bijan.mottahedeh@oracle.com \
    --cc=maz@kernel.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.