From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC Date: Thu, 23 May 2013 12:13:16 +0300 Message-ID: <20130523091316.GJ4725@redhat.com> References: <1368947197-9033-1-git-send-email-vrozenfe@redhat.com> <1368947197-9033-3-git-send-email-vrozenfe@redhat.com> <20130522005046.GA7589@amt.cnet> <643480794.5761686.1369207375451.JavaMail.root@redhat.com> <20130522212330.GA29387@amt.cnet> <519DB4CF.8050003@dlhnet.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marcelo Tosatti , Vadim Rozenfeld , kvm@vger.kernel.org, pl@dlh.net To: Peter Lieven Return-path: Received: from mx1.redhat.com ([209.132.183.28]:19042 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757173Ab3EWJNU (ORCPT ); Thu, 23 May 2013 05:13:20 -0400 Content-Disposition: inline In-Reply-To: <519DB4CF.8050003@dlhnet.de> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, May 23, 2013 at 08:18:55AM +0200, Peter Lieven wrote: > On 22.05.2013 23:23, Marcelo Tosatti wrote: > >On Wed, May 22, 2013 at 03:22:55AM -0400, Vadim Rozenfeld wrote: > >> > >> > >>----- Original Message ----- > >>From: "Marcelo Tosatti" > >>To: "Vadim Rozenfeld" > >>Cc: kvm@vger.kernel.org, gleb@redhat.com, pl@dlh.net > >>Sent: Wednesday, May 22, 2013 10:50:46 AM > >>Subject: Re: [RFC PATCH v2 2/2] add support for Hyper-V invariant TSC > >> > >>On Sun, May 19, 2013 at 05:06:37PM +1000, Vadim Rozenfeld wrote: > >>>The following patch allows to activate a partition reference > >>>time enlightenment that is based on the host platform's support > >>>for an Invariant Time Stamp Counter (iTSC). > >>>NOTE: This code will survive migration due to lack of VM stop/resume > >>>handlers, when offset, scale and sequence should be > >>>readjusted. > >>> > >>>--- > >>> arch/x86/kvm/x86.c | 6 +++++- > >>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>> > >>>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > >>>index 9645dab..b423fe4 100644 > >>>--- a/arch/x86/kvm/x86.c > >>>+++ b/arch/x86/kvm/x86.c > >>>@@ -1838,7 +1838,6 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data) > >>> u64 gfn; > >>> unsigned long addr; > >>> HV_REFERENCE_TSC_PAGE tsc_ref; > >>>- tsc_ref.TscSequence = 0; > >>> if (!(data & HV_X64_MSR_TSC_REFERENCE_ENABLE)) { > >>> kvm->arch.hv_tsc_page = data; > >>> break; > >>>@@ -1848,6 +1847,11 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data) > >>> HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT); > >>> if (kvm_is_error_hva(addr)) > >>> return 1; > >>>+ tsc_ref.TscSequence = > >>>+ boot_cpu_has(X86_FEATURE_CONSTANT_TSC) ? 1 : 0; > >> > >>1) You want NONSTOP_TSC (see 40fb1715 commit) which matches INVARIANT TSC. > >>[VR] > >>Thank you for reviewing. Will fix it. > >>2) TscSequence should increase? > >>"This field serves as a sequence number that is incremented whenever..." > >>[VR] > >>Yes, on every VM resume, including migration. After migration we also need > >>to recalculate scale and adjust offset. > >>3) 0xFFFFFFFF is the value for invalid source of reference time? > >>[VR] Yes, on boot-up. In this case guest will go with PMTimer (not sure about HPET > >>but I can check). But if we set sequence to 0xFFFFFFFF after migration - it's probably will not work. > > > >"Reference TSC during Save and Restore and Migration > > > >To address migration scenarios to physical platforms that do not support > >iTSC, the TscSequence field is used. In the event that a guest partition > >is migrated from an iTSC capable host to a non-iTSC capable host, the > >hypervisor sets TscSequence to the special value of 0xFFFFFFFF, which > >directs the guest operating system to fall back to a different clock > >source (for example, the virtual PM timer)." > > > >Why it would not/does not work after migration? > > > > > > what exactly do we heed the reference TSC for? the reference counter alone works great and it seems > that there is a lot of trouble and crash possibilities involved with the referece tsc. > Reference TSC is even faster. There should be no crashed with proper implementation. -- Gleb.