From: Glauber Costa <glommer@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Zachary Amsden <zamsden@redhat.com>,
kvm@vger.kernel.org, Avi Kivity <avi@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
John Stultz <johnstul@us.ibm.com>,
linux-kernel@vger.kernel.org
Subject: Re: [KVM timekeeping 25/35] Add clock catchup mode
Date: Wed, 25 Aug 2010 20:38:05 -0300 [thread overview]
Message-ID: <20100825233805.GA2985@mothafucka.localdomain> (raw)
In-Reply-To: <20100825220134.GA3322@amt.cnet>
On Wed, Aug 25, 2010 at 07:01:34PM -0300, Marcelo Tosatti wrote:
> On Wed, Aug 25, 2010 at 10:48:20AM -1000, Zachary Amsden wrote:
> > On 08/25/2010 07:27 AM, Marcelo Tosatti wrote:
> > >On Thu, Aug 19, 2010 at 10:07:39PM -1000, Zachary Amsden wrote:
> > >>Make the clock update handler handle generic clock synchronization,
> > >>not just KVM clock. We add a catchup mode which keeps passthrough
> > >>TSC in line with absolute guest TSC.
> > >>
> > >>Signed-off-by: Zachary Amsden<zamsden@redhat.com>
> > >>---
> > >> arch/x86/include/asm/kvm_host.h | 1 +
> > >> arch/x86/kvm/x86.c | 55 ++++++++++++++++++++++++++------------
> > >> 2 files changed, 38 insertions(+), 18 deletions(-)
> > >>
> > >> kvm_x86_ops->vcpu_load(vcpu, cpu);
> > >>- if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
> > >>+ if (unlikely(vcpu->cpu != cpu) || vcpu->arch.tsc_rebase) {
> > >> /* Make sure TSC doesn't go backwards */
> > >> s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
> > >> native_read_tsc() - vcpu->arch.last_host_tsc;
> > >> if (tsc_delta< 0)
> > >> mark_tsc_unstable("KVM discovered backwards TSC");
> > >>- if (check_tsc_unstable())
> > >>+ if (check_tsc_unstable()) {
> > >> kvm_x86_ops->adjust_tsc_offset(vcpu, -tsc_delta);
> > >>- kvm_migrate_timers(vcpu);
> > >>+ kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
> > >>+ }
> > >>+ if (vcpu->cpu != cpu)
> > >>+ kvm_migrate_timers(vcpu);
> > >> vcpu->cpu = cpu;
> > >>+ vcpu->arch.tsc_rebase = 0;
> > >> }
> > >> }
> > >>
> > >>@@ -1947,6 +1961,12 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> > >> kvm_x86_ops->vcpu_put(vcpu);
> > >> kvm_put_guest_fpu(vcpu);
> > >> vcpu->arch.last_host_tsc = native_read_tsc();
> > >>+
> > >>+ /* For unstable TSC, force compensation and catchup on next CPU */
> > >>+ if (check_tsc_unstable()) {
> > >>+ vcpu->arch.tsc_rebase = 1;
> > >>+ kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
> > >>+ }
> > >The mix between catchup,trap versus stable,unstable TSC is confusing and
> > >difficult to grasp. Can you please introduce all the infrastructure
> > >first, then control usage of them in centralized places? Examples:
> > >
> > >+static void kvm_update_tsc_trapping(struct kvm *kvm)
> > >+{
> > >+ int trap, i;
> > >+ struct kvm_vcpu *vcpu;
> > >+
> > >+ trap = check_tsc_unstable()&& atomic_read(&kvm->online_vcpus)> 1;
> > >+ kvm_for_each_vcpu(i, vcpu, kvm)
> > >+ kvm_x86_ops->set_tsc_trap(vcpu, trap&& !vcpu->arch.time_page);
> > >+}
> > >
> > >+ /* For unstable TSC, force compensation and catchup on next CPU */
> > >+ if (check_tsc_unstable()) {
> > >+ vcpu->arch.tsc_rebase = 1;
> > >+ kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
> > >+ }
> > >
> > >
> > >kvm_guest_time_update is becoming very confusing too. I understand this
> > >is due to the many cases its dealing with, but please make it as simple
> > >as possible.
> >
> > I tried to comment as best as I could. I think the whole
> > "kvm_update_tsc_trapping" thing is probably a poor design choice.
> > It works, but it's thoroughly unintelligible right now without
> > spending some days figuring out why.
> >
> > I'll rework the tail series of patches to try to make them more clear.
> >
> > >+ /*
> > >+ * If we are trapping and no longer need to, use catchup to
> > >+ * ensure passthrough TSC will not be less than trapped TSC
> > >+ */
> > >+ if (vcpu->tsc_mode == TSC_MODE_PASSTHROUGH&& vcpu->tsc_trapping&&
> > >+ ((this_tsc_khz<= v->kvm->arch.virtual_tsc_khz || kvmclock))) {
> > >+ catchup = 1;
> > >
> > >What, TSC trapping with kvmclock enabled?
> >
> > Transitioning to use of kvmclock after a cold boot means we may have
> > been trapping and now we will not be.
> >
> > >For both catchup and trapping the resolution of the host clock is
> > >important, as Glauber commented for kvmclock. Can you comment on the
> > >problems that arrive from a low res clock for both modes?
> > >
> > >Similarly for catchup mode, the effect of exit frequency. No need for
> > >any guarantees?
> >
> > The scheduler will do something to get an IRQ at whatever resolution
> > it uses for it's timeslice. That guarantees an exit per timeslice,
> > so we'll never be behind by more than one slice while scheduling.
> > While not scheduling, we're dormant anyway, waiting on either an IRQ
> > or shared memory variable change. Local timers could end up behind
> > when dormant.
> >
> > We may need a hack to accelerate firing of timers in such a case, or
> > perhaps bounds on when to use catchup mode and when to not.
>
> What about emulating rdtsc with low res clock?
>
> "The RDTSC instruction reads the time-stamp counter and is guaranteed to
> return a monotonically increasing unique value whenever executed, except
> for a 64-bit counter wraparound."
>
This is bad semantics, IMHO. It is a totally different behaviour than the
one guest users would expect.
next prev parent reply other threads:[~2010-08-25 23:38 UTC|newest]
Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-20 8:07 KVM timekeeping and TSC virtualization Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 01/35] Drop vm_init_tsc Zachary Amsden
2010-08-20 16:54 ` Glauber Costa
2010-08-20 8:07 ` [KVM timekeeping 02/35] Convert TSC writes to TSC offset writes Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 03/35] Move TSC offset writes to common code Zachary Amsden
2010-08-20 17:06 ` Glauber Costa
2010-08-24 0:51 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 04/35] Fix SVM VMCB reset Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 05/35] Move TSC reset out of vmcb_init Zachary Amsden
2010-08-20 17:08 ` Glauber Costa
2010-08-24 0:52 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 06/35] TSC reset compensation Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 07/35] Make cpu_tsc_khz updates use local CPU Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 08/35] Warn about unstable TSC Zachary Amsden
2010-08-20 17:28 ` Glauber Costa
2010-08-24 0:56 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 09/35] Unify TSC logic Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 10/35] Fix deep C-state TSC desynchronization Zachary Amsden
2010-08-20 17:30 ` Glauber Costa
2010-09-14 9:10 ` Jan Kiszka
2010-09-14 9:27 ` Avi Kivity
2010-09-14 10:40 ` Jan Kiszka
2010-09-14 10:47 ` Avi Kivity
2010-09-14 19:32 ` Zachary Amsden
2010-09-14 22:26 ` Jan Kiszka
2010-09-14 23:40 ` Zachary Amsden
2010-09-15 5:34 ` Jan Kiszka
2010-09-15 7:55 ` Avi Kivity
2010-09-15 8:04 ` Jan Kiszka
2010-09-15 12:29 ` Glauber Costa
2010-09-15 4:07 ` Zachary Amsden
2010-09-15 8:09 ` Jan Kiszka
2010-09-15 12:32 ` Glauber Costa
2010-09-15 18:27 ` Jan Kiszka
2010-09-17 22:09 ` Zachary Amsden
2010-09-17 22:31 ` Zachary Amsden
2010-09-18 23:53 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 11/35] Add helper functions for time computation Zachary Amsden
2010-08-20 17:34 ` Glauber Costa
2010-08-24 0:58 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 12/35] Robust TSC compensation Zachary Amsden
2010-08-20 17:40 ` Glauber Costa
2010-08-24 1:01 ` Zachary Amsden
2010-08-24 21:33 ` Daniel Verkamp
2010-08-20 8:07 ` [KVM timekeeping 13/35] Perform hardware_enable in CPU_STARTING callback Zachary Amsden
2010-08-27 16:32 ` Jan Kiszka
2010-08-27 23:43 ` Zachary Amsden
2010-08-30 9:10 ` Jan Kiszka
2010-08-20 8:07 ` [KVM timekeeping 14/35] Add clock sync request to hardware enable Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 15/35] Move scale_delta into common header Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 16/35] Fix a possible backwards warp of kvmclock Zachary Amsden
2011-09-02 18:34 ` Philipp Hahn
2011-09-05 14:06 ` [BUG, PATCH-2.6.32] " Philipp Hahn
2011-09-12 11:32 ` Marcelo Tosatti
2010-08-20 8:07 ` [KVM timekeeping 17/35] Implement getnsboottime kernel API Zachary Amsden
2010-08-20 18:39 ` john stultz
2010-08-20 23:37 ` Zachary Amsden
2010-08-21 0:02 ` john stultz
2010-08-21 0:52 ` Zachary Amsden
2010-08-21 1:04 ` john stultz
2010-08-21 1:22 ` Zachary Amsden
2010-08-27 18:05 ` Jan Kiszka
2010-08-27 23:48 ` Zachary Amsden
2010-08-30 18:07 ` Jan Kiszka
2010-08-20 8:07 ` [KVM timekeeping 18/35] Use getnsboottime in KVM Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 19/35] Add timekeeping documentation Zachary Amsden
2010-08-20 17:50 ` Glauber Costa
2010-08-20 8:07 ` [KVM timekeeping 20/35] Make math work for other scales Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 21/35] Track max tsc_khz Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 22/35] Track tsc last write in vcpu Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 23/35] Set initial TSC rate conversion factors Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 24/35] Timer request function renaming Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 25/35] Add clock catchup mode Zachary Amsden
2010-08-25 17:27 ` Marcelo Tosatti
2010-08-25 20:48 ` Zachary Amsden
2010-08-25 22:01 ` Marcelo Tosatti
2010-08-25 23:38 ` Glauber Costa [this message]
2010-08-26 0:17 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 26/35] Catchup slower TSC to guest rate Zachary Amsden
2010-09-07 3:44 ` Dong, Eddie
2010-09-07 22:14 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 27/35] Add TSC trapping Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 28/35] Unstable TSC write compensation Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 29/35] TSC overrun protection Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 30/35] IOCTL for setting TSC rate Zachary Amsden
2010-08-20 17:56 ` Glauber Costa
2010-08-21 16:11 ` Arnd Bergmann
2010-08-20 8:07 ` [KVM timekeeping 31/35] Exit conditions for TSC trapping Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 32/35] Entry " Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 33/35] Indicate reliable TSC in kvmclock Zachary Amsden
2010-08-20 17:45 ` Glauber Costa
2010-08-24 1:14 ` Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 34/35] Remove dead code Zachary Amsden
2010-08-20 8:07 ` [KVM timekeeping 35/35] Add some debug stuff Zachary Amsden
2010-08-20 13:26 ` KVM timekeeping and TSC virtualization David S. Ahern
2010-08-20 23:24 ` Zachary Amsden
2010-08-22 1:32 ` David S. Ahern
2010-08-24 1:44 ` Zachary Amsden
2010-08-24 3:04 ` David S. Ahern
2010-08-24 5:47 ` Zachary Amsden
2010-08-24 13:32 ` David S. Ahern
2010-08-24 23:01 ` Zachary Amsden
2010-08-25 16:55 ` Marcelo Tosatti
2010-08-25 20:32 ` Zachary Amsden
2010-08-24 22:13 ` Marcelo Tosatti
2010-08-25 4:04 ` Zachary Amsden
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=20100825233805.GA2985@mothafucka.localdomain \
--to=glommer@redhat.com \
--cc=avi@redhat.com \
--cc=johnstul@us.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=tglx@linutronix.de \
--cc=zamsden@redhat.com \
/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).