From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59481) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2hJ5-00065v-G7 for qemu-devel@nongnu.org; Fri, 04 Nov 2016 12:25:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2hJ1-0008V8-Ge for qemu-devel@nongnu.org; Fri, 04 Nov 2016 12:25:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50042) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2hJ1-0008UL-9J for qemu-devel@nongnu.org; Fri, 04 Nov 2016 12:25:27 -0400 Date: Fri, 4 Nov 2016 14:24:23 -0200 From: Marcelo Tosatti Message-ID: <20161104162420.GC22546@amt.cnet> References: <20161104094322.GA16930@amt.cnet> <20161104152522.GC5388@potion> <1c69a083-eef0-8fa0-0e74-5a4e25a066a0@redhat.com> <20161104154827.GD5388@potion> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161104154827.GD5388@potion> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [QEMU PATCH] kvmclock: advance clock by time window between vm_stop and pre_save List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Radim =?utf-8?B?S3LEjW3DocWZ?= Cc: Paolo Bonzini , kvm@vger.kernel.org, qemu-devel , "Dr. David Alan Gilbert" , Juan Quintela , Eduardo Habkost , Roman Kagan On Fri, Nov 04, 2016 at 04:48:28PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 wro= te: > 2016-11-04 16:33+0100, Paolo Bonzini: > > On 04/11/2016 16:25, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > >>> > =20 > >>> > + if (s->advance_clock && s->clock + s->advance_clock > s-= >clock) { > >>> > + s->clock +=3D s->advance_clock; > >>> > + s->advance_clock =3D 0; > >>> > + } > >> Can't the advance_clock added to the migrated KVMClockState instead = of > >> passing it as another parameter? > >>=20 > >> (It is sad that we can't just query KVMClockState in kvmclock_pre_sa= ve > >> because of the Linux bug.) > >=20 > > What Linux bug? The one that makes us use kvmclock_current_nsec? >=20 > No, the one that forced Marcelo to add the 10 minute limit to the > advance_clock.=20 Overflow when reading clocksource, in the timer interrupt. > We wouldn't need this advance_clock hack if we could > just call KVM_GET_CLOCK like we did before 00f4d64ee76e ("kvmclock: > clock should count only if vm is running"). People have requested that CLOCK_MONOTONIC stops when=20 vm suspends. > > It should work with 4.9-rc (well, once Linus applies my pull request)= . > > 4.9-rc will not return ktime_get_ns for KVM_GET_CLOCK; it will return > > the raw value from the kernel timekeeper. > >=20 > > I'm thinking that we should add a KVM capability for this, and skip > > kvmclock_current_nsec if the capability is present. The first part i= s > > trivial, so we can do it even during Linux rc period. >=20 > I agree, that sounds like a nice improvement. I fail to see how this (KVM_GET_CLOCK using raw value (vs NTP corrected) relates to clocksource overflow and CLOCK_MONOTONIC stop requests. ----- Todo list (collective???): 1) Implement suggestion to Roman:=20 "Subject: Re: [PATCH] x86/kvm: fix condition to update kvm master clocks" to handle the time backwards when updating masterclock=20 from kernel clock. 2) Review TSC scaling support (make sure scaled TSC=20 is propagated properly everywhere). 3) Enable migration with invariant TSC. 4) Fullvirt fix for local APIC deadline timer bug (BTW Paolo Windows is also vulnerable right).