From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44654) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X78ed-0004jM-PI for qemu-devel@nongnu.org; Tue, 15 Jul 2014 15:44:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X78eY-0003aK-2o for qemu-devel@nongnu.org; Tue, 15 Jul 2014 15:44:47 -0400 Received: from mail-qc0-x229.google.com ([2607:f8b0:400d:c01::229]:62350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X78eX-0003aB-V0 for qemu-devel@nongnu.org; Tue, 15 Jul 2014 15:44:42 -0400 Received: by mail-qc0-f169.google.com with SMTP id m20so3563488qcx.14 for ; Tue, 15 Jul 2014 12:44:41 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <53C584A5.3070206@redhat.com> Date: Tue, 15 Jul 2014 21:44:37 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1400253321-9239-1-git-send-email-agraf@suse.de> In-Reply-To: <1400253321-9239-1-git-send-email-agraf@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] kvmclock: Ensure time in migration never goes backward List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf , qemu-devel@nongnu.org Cc: mtosatti@redhat.com, kvm@vger.kernel.org Il 16/05/2014 17:15, Alexander Graf ha scritto: > When we migrate we ask the kernel about its current belief on what the guest > time would be. However, I've seen cases where the kvmclock guest structure > indicates a time more recent than the kvm returned time. > > To make sure we never go backwards, calculate what the guest would have seen > as time at the point of migration and use that value instead of the kernel > returned one when it's more recent. > > While the underlying bug is supposedly fixed on newer KVM versions, it doesn't > hurt to base the view of the kvmclock after migration on the same foundation > in host as well as guest. > > Signed-off-by: Alexander Graf > > --- > > v1 -> v2: > > - always use guest structure when available > --- > hw/i386/kvm/clock.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c > index 892aa02..6f4ed28a 100644 > --- a/hw/i386/kvm/clock.c > +++ b/hw/i386/kvm/clock.c > @@ -14,6 +14,7 @@ > */ > > #include "qemu-common.h" > +#include "qemu/host-utils.h" > #include "sysemu/sysemu.h" > #include "sysemu/kvm.h" > #include "hw/sysbus.h" > @@ -34,6 +35,47 @@ typedef struct KVMClockState { > bool clock_valid; > } KVMClockState; > > +struct pvclock_vcpu_time_info { > + uint32_t version; > + uint32_t pad0; > + uint64_t tsc_timestamp; > + uint64_t system_time; > + uint32_t tsc_to_system_mul; > + int8_t tsc_shift; > + uint8_t flags; > + uint8_t pad[2]; > +} __attribute__((__packed__)); /* 32 bytes */ > + > +static uint64_t kvmclock_current_nsec(KVMClockState *s) > +{ > + CPUState *cpu = first_cpu; > + CPUX86State *env = cpu->env_ptr; > + hwaddr kvmclock_struct_pa = env->system_time_msr & ~1ULL; > + uint64_t migration_tsc = env->tsc; > + struct pvclock_vcpu_time_info time; > + uint64_t delta; > + uint64_t nsec_lo; > + uint64_t nsec_hi; > + uint64_t nsec; > + > + if (!(env->system_time_msr & 1ULL)) { > + /* KVM clock not active */ > + return 0; > + } > + > + cpu_physical_memory_read(kvmclock_struct_pa, &time, sizeof(time)); > + > + delta = migration_tsc - time.tsc_timestamp; > + if (time.tsc_shift < 0) { > + delta >>= -time.tsc_shift; > + } else { > + delta <<= time.tsc_shift; > + } > + > + mulu64(&nsec_lo, &nsec_hi, delta, time.tsc_to_system_mul); > + nsec = (nsec_lo >> 32) | (nsec_hi << 32); > + return nsec + time.system_time; > +} > > static void kvmclock_vm_state_change(void *opaque, int running, > RunState state) > @@ -45,9 +87,15 @@ static void kvmclock_vm_state_change(void *opaque, int running, > > if (running) { > struct kvm_clock_data data; > + uint64_t time_at_migration = kvmclock_current_nsec(s); > > s->clock_valid = false; > > + /* We can't rely on the migrated clock value, just discard it */ > + if (time_at_migration) { > + s->clock = time_at_migration; > + } > + > data.clock = s->clock; > data.flags = 0; > ret = kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data); > I'm going to revert this patch for 2.1-rc3, since the dependent patch "kvmclock: Ensure proper env->tsc value for kvmclock_current_nsec calculation" causes a hang during migration. Paolo