From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [QEMU PATCH] kvmclock: advance clock by time window between vm_stop and pre_save Date: Fri, 4 Nov 2016 14:24:23 -0200 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-Transfer-Encoding: 8bit Cc: Paolo Bonzini , kvm@vger.kernel.org, qemu-devel , "Dr. David Alan Gilbert" , Juan Quintela , Eduardo Habkost , Roman Kagan To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47568 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933214AbcKDQZ0 (ORCPT ); Fri, 4 Nov 2016 12:25:26 -0400 Content-Disposition: inline In-Reply-To: <20161104154827.GD5388@potion> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Nov 04, 2016 at 04:48:28PM +0100, Radim Krčmář wrote: > 2016-11-04 16:33+0100, Paolo Bonzini: > > On 04/11/2016 16:25, Radim Krčmář wrote: > >>> > > >>> > + if (s->advance_clock && s->clock + s->advance_clock > s->clock) { > >>> > + s->clock += s->advance_clock; > >>> > + s->advance_clock = 0; > >>> > + } > >> Can't the advance_clock added to the migrated KVMClockState instead of > >> passing it as another parameter? > >> > >> (It is sad that we can't just query KVMClockState in kvmclock_pre_save > >> because of the Linux bug.) > > > > What Linux bug? The one that makes us use kvmclock_current_nsec? > > No, the one that forced Marcelo to add the 10 minute limit to the > advance_clock. 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 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. > > > > 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 is > > trivial, so we can do it even during Linux rc period. > > 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: "Subject: Re: [PATCH] x86/kvm: fix condition to update kvm master clocks" to handle the time backwards when updating masterclock from kernel clock. 2) Review TSC scaling support (make sure scaled TSC 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). 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).