qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Laurent Vivier <lvivier@redhat.com>,
	Peter Crosthwaite <crosthwaitepeter@gmail.com>,
	Christopher Covington <cov@codeaurora.org>,
	Alistair Francis <alistair.francis@xilinx.com>
Subject: Re: [Qemu-devel] [PATCH] target-arm: Use common CPU cycle infrastructure
Date: Mon, 5 Oct 2015 16:27:28 +0200	[thread overview]
Message-ID: <561288D0.9000205@redhat.com> (raw)
In-Reply-To: <CAFEAcA8BN+6FO--uEMx6rsFrJbToNWE7Cpo4Tj-gGx6Fjen5kQ@mail.gmail.com>



On 05/10/2015 16:11, Peter Maydell wrote:
> > > OTOH various non-x86 things do use the closely related cpu_get_real_ticks(),
> > > and the implementation of cpu_get_ticks() is very closely related to
> > > the other clock code in cpus.c.
> >
> > cpu_get_real_ticks() is returning the host cycle counter;
> > cpu_get_ticks() is stopping/resuming it when the VM is stopped/resumed.
> >  In other words, cpu_get_real_ticks() is to cpu_get_ticks() what
> > QEMU_CLOCK_REALTIME is to QEMU_CLOCK_VIRTUAL.
>
> ...but it seems wrong to have anything in the simulation care
> about the host cycle counter,

I agree; instruction count is much better.  Unfortunately, -icount has a
relatively hefty performance penalty.  It is also common that code using
PMCCNTR/RDTSC wants the increment between two reads to be large-ish and
at least remotely related to the number of instructions that were
executed in between.

I've also used rdtsc in the guest from time to time to benchmark changes
to TCG. Having it map to the host cycle counter is somewhat useful for
that purpose, though one might say this usecase is niche.

> especially since on some hosts
> the underlying implementation is terrible.

I remember seeing a bug where this terrible implementation caused
divisions by zero on the host.  The default implementation in
include/qemu/timer.h should be changed to
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL).

Note that in practice this terrible implementation is only used on
ARM/AArch64.  Is PMCCNTR or something similar accessible from userspace?
 I guess no, since even write access is enabled via PMUSERENR (and in
general PMUSERENR ought to be false on a preemptive-multitasking OS).

In the end I guess qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) would work just
as well for PMCCNTR, but I think non-Linux OSes still have a relatively
slow clock_gettime() so there is still an advantage in using
cpu_get_ticks().

Paolo

ps: on x86, a long time ago rdtsc was reliable and clock_gettime() was
slow, so it was a no-brainer for benchmarks and the like; then rdtsc
became unreliable and clock_gettime() became fast on Linux; but now at
least on new machines rdtsc is usually reliable again.

  reply	other threads:[~2015-10-05 14:27 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-30 18:14 [Qemu-devel] [RFC 1/5] arm64: Add PMINTENCLR_EL1 Christopher Covington
2015-04-30 18:14 ` [Qemu-devel] [RFC 2/5] arm64: Add PMOVSCLR_EL0 register Christopher Covington
2015-04-30 18:14 ` [Qemu-devel] [RFC 3/5] arm64: Add PMUSERENR_EL0 register Christopher Covington
2015-04-30 18:14 ` [Qemu-devel] [RFC 4/5] arm64: Unmask PMU bits in debug feature register Christopher Covington
2015-04-30 18:14 ` [Qemu-devel] [RFC 5/5] arm: Simplify cycle counter Christopher Covington
2015-04-30 18:27   ` Peter Maydell
2015-04-30 21:33     ` Christopher Covington
2015-04-30 22:02       ` Peter Maydell
2015-05-04  9:54         ` Paolo Bonzini
2015-05-01  1:24   ` Peter Crosthwaite
2015-05-01 14:35     ` Christopher Covington
2015-05-06 14:05       ` Peter Crosthwaite
2015-05-06 15:38         ` Peter Maydell
2015-09-24 19:43   ` [Qemu-devel] [PATCH] target-arm: Use common CPU cycle infrastructure Christopher Covington
2015-09-28 22:05     ` Alistair Francis
2015-09-29 14:07       ` Christopher Covington
2015-10-02 16:44         ` Christopher Covington
2015-10-02 16:56           ` Peter Maydell
2015-10-02 17:25             ` Peter Crosthwaite
2015-10-02 18:08               ` Peter Maydell
2015-10-02 18:14                 ` Peter Crosthwaite
2015-10-02 19:25               ` Christopher Covington
2015-10-02 19:56                 ` Peter Crosthwaite
2015-10-02 20:48                   ` Christopher Covington
2015-10-02 22:41                     ` Peter Maydell
2015-10-05 14:09                       ` Paolo Bonzini
2015-10-05 14:11                         ` Peter Maydell
2015-10-05 14:27                           ` Paolo Bonzini [this message]
2015-10-06 12:49                             ` Peter Maydell
2015-10-06 12:58                               ` Paolo Bonzini
2015-10-06 13:06                                 ` Peter Maydell
2015-10-06 13:10                                   ` Paolo Bonzini
2015-10-13 20:53     ` Peter Maydell
2015-10-14 12:10       ` Christopher Covington

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=561288D0.9000205@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=alistair.francis@xilinx.com \
    --cc=cov@codeaurora.org \
    --cc=crosthwaitepeter@gmail.com \
    --cc=lvivier@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).