From patchwork Sat Dec 10 17:21:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Tosatti X-Patchwork-Id: 9469389 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2468760231 for ; Sat, 10 Dec 2016 17:28:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1023F28466 for ; Sat, 10 Dec 2016 17:28:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 02FEF2851B; Sat, 10 Dec 2016 17:28:54 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6581728466 for ; Sat, 10 Dec 2016 17:28:53 +0000 (UTC) Received: from localhost ([::1]:52562 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cFlS8-0002ya-HM for patchwork-qemu-devel@patchwork.kernel.org; Sat, 10 Dec 2016 12:28:52 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49845) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cFlP2-0000zQ-CJ for qemu-devel@nongnu.org; Sat, 10 Dec 2016 12:25:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cFlOz-00038t-HG for qemu-devel@nongnu.org; Sat, 10 Dec 2016 12:25:40 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37388) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cFlOz-00038W-9W for qemu-devel@nongnu.org; Sat, 10 Dec 2016 12:25:37 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 12B33744; Sat, 10 Dec 2016 17:25:36 +0000 (UTC) Received: from amt.cnet (vpn1-4-145.gru2.redhat.com [10.97.4.145]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uBAHPYxO021605; Sat, 10 Dec 2016 12:25:34 -0500 Received: from amt.cnet (localhost [127.0.0.1]) by amt.cnet (Postfix) with ESMTP id D61161009C8; Sat, 10 Dec 2016 15:24:40 -0200 (BRST) Received: (from marcelo@localhost) by amt.cnet (8.14.7/8.14.7/Submit) id uBAHOeDU008463; Sat, 10 Dec 2016 15:24:40 -0200 Message-Id: <20161210172324.482367805@redhat.com> User-Agent: quilt/0.60-1 Date: Sat, 10 Dec 2016 15:21:50 -0200 From: Marcelo Tosatti To: kvm@vger.kernel.org References: <20161210172148.361793326@redhat.com> Content-Disposition: inline; filename=kvmclock-advance-clock.patch X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Sat, 10 Dec 2016 17:25:36 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [qemu patch V4 2/2] kvmclock: reduce kvmclock difference on migration X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Marcelo Tosatti , Eduardo Habkost , Juan Quintela , Radim Krcmar , qemu-devel@nongnu.org, Dr David Alan Gilbert , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Check for KVM_CAP_ADJUST_CLOCK capability KVM_CLOCK_TSC_STABLE, which indicates that KVM_GET_CLOCK returns a value as seen by the guest at that moment. For new machine types, use this value rather than reading from guest memory. This reduces kvmclock difference on migration from 5s to 0.1s (when max_downtime == 5s). Note: pre_save contradicts the following comment /* * If the VM is stopped, declare the clock state valid to * avoid re-reading it on next vmsave (which would return * a different value). Will be reset when the VM is continued. */ But the comment is bogus: vm_state_change is never called twice in a row with running=0 or running=1. Signed-off-by: Marcelo Tosatti Reviewed-by: Eduardo Habkost Reviewed-by: Pankaj Gupta --- hw/i386/kvm/clock.c | 107 ++++++++++++++++++++++++++++++++++++++++++------- include/hw/i386/pc.h | 5 ++ target-i386/kvm.c | 7 +++ target-i386/kvm_i386.h | 1 4 files changed, 106 insertions(+), 14 deletions(-) v2: - improve variable names (Juan) - consolidate code on kvm_get_clock function (Paolo) - return mach_use_reliable_get_clock from needed function (Paolo) v3: - simplify check for src_use_reliable_get_clock (Eduardo) - move src_use_reliable_get_clock initialization to realize (Eduardo) v4: - have kvm_get_clock and clock_is_reliable assignments synchronized (Eduardo) - add comment to the reasoning of kvmclock_pre_save (Eduardo) Index: qemu-mig-advance-clock/hw/i386/kvm/clock.c =================================================================== --- qemu-mig-advance-clock.orig/hw/i386/kvm/clock.c 2016-11-17 15:07:11.220632761 -0200 +++ qemu-mig-advance-clock/hw/i386/kvm/clock.c 2016-12-10 13:57:58.115983966 -0200 @@ -36,6 +36,13 @@ uint64_t clock; bool clock_valid; + + /* whether machine type supports reliable get clock */ + bool mach_use_reliable_get_clock; + + /* whether the 'clock' value was obtained in a host with + * reliable KVM_GET_CLOCK */ + bool clock_is_reliable; } KVMClockState; struct pvclock_vcpu_time_info { @@ -81,6 +88,19 @@ return nsec + time.system_time; } +static uint64_t kvm_get_clock(void) +{ + struct kvm_clock_data data; + int ret; + + ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data); + if (ret < 0) { + fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret)); + abort(); + } + return data.clock; +} + static void kvmclock_vm_state_change(void *opaque, int running, RunState state) { @@ -91,15 +111,23 @@ if (running) { struct kvm_clock_data data = {}; - uint64_t time_at_migration = kvmclock_current_nsec(s); + uint64_t pvclock_via_mem = 0; - s->clock_valid = false; + /* + * If the host where s->clock was read did not support reliable + * KVM_GET_CLOCK, read kvmclock value from memory. + */ + if (!s->clock_is_reliable) { + pvclock_via_mem = kvmclock_current_nsec(s); + } - /* We can't rely on the migrated clock value, just discard it */ - if (time_at_migration) { - s->clock = time_at_migration; + /* We can't rely on the saved clock value, just discard it */ + if (pvclock_via_mem) { + s->clock = pvclock_via_mem; } + s->clock_valid = false; + data.clock = s->clock; ret = kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data); if (ret < 0) { @@ -120,8 +148,6 @@ } } } else { - struct kvm_clock_data data; - int ret; if (s->clock_valid) { return; @@ -129,13 +155,11 @@ kvm_synchronize_all_tsc(); - ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data); - if (ret < 0) { - fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret)); - abort(); - } - s->clock = data.clock; - + s->clock = kvm_get_clock(); + /* any code that sets s->clock needs to ensure clock_is_reliable + * is correctly set. + */ + s->clock_is_reliable = kvm_has_adjust_clock_stable(); /* * If the VM is stopped, declare the clock state valid to * avoid re-reading it on next vmsave (which would return @@ -149,25 +173,80 @@ { KVMClockState *s = KVM_CLOCK(dev); + if (kvm_has_adjust_clock_stable()) { + s->clock_is_reliable = true; + } + qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s); } +static bool kvmclock_clock_is_reliable_needed(void *opaque) +{ + KVMClockState *s = opaque; + + return s->mach_use_reliable_get_clock; +} + +static const VMStateDescription kvmclock_reliable_get_clock = { + .name = "kvmclock/clock_is_reliable", + .version_id = 1, + .minimum_version_id = 1, + .needed = kvmclock_clock_is_reliable_needed, + .fields = (VMStateField[]) { + VMSTATE_BOOL(clock_is_reliable, KVMClockState), + VMSTATE_END_OF_LIST() + } +}; + +/* + * When migrating, read the clock just before migration, + * so that the guest clock counts during the events + * between: + * + * * vm_stop() + * * + * * pre_save() + * + * This reduces kvmclock difference on migration from 5s + * to 0.1s (when max_downtime == 5s), because sending the + * final pages of memory (which happens between vm_stop() + * and pre_save()) takes max_downtime. + */ +static void kvmclock_pre_save(void *opaque) +{ + KVMClockState *s = opaque; + + s->clock = kvm_get_clock(); +} + static const VMStateDescription kvmclock_vmsd = { .name = "kvmclock", .version_id = 1, .minimum_version_id = 1, + .pre_save = kvmclock_pre_save, .fields = (VMStateField[]) { VMSTATE_UINT64(clock, KVMClockState), VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription * []) { + &kvmclock_reliable_get_clock, + NULL } }; +static Property kvmclock_properties[] = { + DEFINE_PROP_BOOL("x-mach-use-reliable-get-clock", KVMClockState, + mach_use_reliable_get_clock, true), + DEFINE_PROP_END_OF_LIST(), +}; + static void kvmclock_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->realize = kvmclock_realize; dc->vmsd = &kvmclock_vmsd; + dc->props = kvmclock_properties; } static const TypeInfo kvmclock_info = { Index: qemu-mig-advance-clock/include/hw/i386/pc.h =================================================================== --- qemu-mig-advance-clock.orig/include/hw/i386/pc.h 2016-11-17 15:07:11.220632761 -0200 +++ qemu-mig-advance-clock/include/hw/i386/pc.h 2016-11-17 15:08:58.096791416 -0200 @@ -370,6 +370,11 @@ #define PC_COMPAT_2_7 \ HW_COMPAT_2_7 \ {\ + .driver = "kvmclock",\ + .property = "x-mach-use-reliable-get-clock",\ + .value = "off",\ + },\ + {\ .driver = TYPE_X86_CPU,\ .property = "l3-cache",\ .value = "off",\ Index: qemu-mig-advance-clock/target-i386/kvm.c =================================================================== --- qemu-mig-advance-clock.orig/target-i386/kvm.c 2016-11-17 15:07:11.221632762 -0200 +++ qemu-mig-advance-clock/target-i386/kvm.c 2016-11-17 15:07:15.867639659 -0200 @@ -117,6 +117,13 @@ return kvm_check_extension(kvm_state, KVM_CAP_X86_SMM); } +bool kvm_has_adjust_clock_stable(void) +{ + int ret = kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK); + + return (ret == KVM_CLOCK_TSC_STABLE); +} + bool kvm_allows_irq0_override(void) { return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing(); Index: qemu-mig-advance-clock/target-i386/kvm_i386.h =================================================================== --- qemu-mig-advance-clock.orig/target-i386/kvm_i386.h 2016-11-17 15:07:11.222632764 -0200 +++ qemu-mig-advance-clock/target-i386/kvm_i386.h 2016-11-17 15:07:15.867639659 -0200 @@ -17,6 +17,7 @@ bool kvm_allows_irq0_override(void); bool kvm_has_smm(void); +bool kvm_has_adjust_clock_stable(void); void kvm_synchronize_all_tsc(void); void kvm_arch_reset_vcpu(X86CPU *cs); void kvm_arch_do_init_vcpu(X86CPU *cs);