From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 1/3] Sync guest viewable TSC when vcpu migrated Date: Sat, 03 Mar 2007 10:47:00 +0200 Message-ID: <45E93604.3080307@qumranet.com> References: <45E853A0.3080207@refactor.fi> <45E85566.3010706@refactor.fi> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Leonard Norrgard Return-path: In-Reply-To: <45E85566.3010706-g2GXA8XeJSExHbG02/KK1g@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Thanks for picking this up! Some comments below. Leonard Norrgard wrote: > Index: linux-2.6/drivers/kvm/svm.c > =================================================================== > --- linux-2.6.orig/drivers/kvm/svm.c 2007-03-02 17:29:57.000000000 +0200 > +++ linux-2.6/drivers/kvm/svm.c 2007-03-02 17:31:15.000000000 +0200 > @@ -598,9 +598,45 @@ > kfree(vcpu->svm); > } > > +#ifdef CONFIG_SMP > +static void ipi_rdtscll(void *arg) > +{ > + u64 *tsc = arg; > + rdtscll(*tsc); > +} > +#endif > + > +/* > + * Switches to specified vcpu, until a matching vcpu_put(), but assumes > + * vcpu mutex is already taken. > + */ > static struct kvm_vcpu *svm_vcpu_load(struct kvm_vcpu *vcpu) > { > - get_cpu(); > + int cpu; > + > + cpu = get_cpu(); > + > +#ifdef CONFIG_SMP > Please avoid such #ifdefs. Non-SMP virtualization capable processors are rare, and smp_call_function_single() is defined for the !CONFIG_SMP case. > + if (vcpu->cpu != cpu) { > + if (vcpu->cpu != -1) { > + u64 tsc_this, tsc_previous; > + > + /* Get TSC value for this and the previous cpu. */ > + rdtscll(tsc_this); > + smp_call_function_single(vcpu->cpu, ipi_rdtscll, > + &tsc_previous, 0, 1); > + > + /* > + * Make sure that the guest sees a monotonically > + * increasing TSC. > + */ > + vcpu->svm->vmcb->control.tsc_offset += > + tsc_previous - tsc_this; > > > Index: linux-2.6/drivers/kvm/vmx.c > =================================================================== > --- linux-2.6.orig/drivers/kvm/vmx.c 2007-03-02 17:30:01.000000000 +0200 > +++ linux-2.6/drivers/kvm/vmx.c 2007-03-02 17:30:17.000000000 +0200 > @@ -200,6 +200,14 @@ > #endif > } > > +#ifdef CONFIG_SMP > +static void ipi_rdtscll(void *arg) > +{ > + u64 *tsc = arg; > + rdtscll(*tsc); > +} > +#endif > + > /* > * Switches to specified vcpu, until a matching vcpu_put(), but assumes > * vcpu mutex is already taken. > @@ -230,6 +238,25 @@ > struct descriptor_table dt; > unsigned long sysenter_esp; > > +#ifdef CONFIG_SMP > + if (vcpu->cpu != -1) { > + u64 tsc_this, tsc_previous, guest_tsc_offset; > + > + /* Get TSC value for this and the previous cpu. */ > + rdtscll(tsc_this); > + smp_call_function_single(vcpu->cpu, ipi_rdtscll, > + &tsc_previous, 0, 1); > + > + /* > + * Make sure that the guest sees a monotonically > + * increasing TSC. > + */ > + guest_tsc_offset = vmcs_read64(TSC_OFFSET); > + vmcs_write64(TSC_OFFSET, guest_tsc_offset + > + tsc_previous - tsc_this); > + } > +#endif > + > The vmx code already has an IPI (in vcpu_clear), so the two should be merged. IPIs are very expensive. Perhaps call a vmx_vcpu_migrate IPI, which calls __vcpu_clear() and loads the tsc. You can add a tsc_previous to struct kvm_vcpu so you have somewhere to store it. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV