From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 4/6] KVM: SVM: Propagate requested TSC frequency on vcpu init Date: Thu, 24 Mar 2011 12:30:38 +0200 Message-ID: <4D8B1D4E.10008@redhat.com> References: <1300952424-32014-1-git-send-email-joerg.roedel@amd.com> <1300952424-32014-5-git-send-email-joerg.roedel@amd.com> <4D8B171D.8080408@redhat.com> <20110324102129.GF18867@8bytes.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Joerg Roedel , Marcelo Tosatti , Zachary Amsden , kvm@vger.kernel.org To: Joerg Roedel Return-path: Received: from mx1.redhat.com ([209.132.183.28]:64442 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933446Ab1CXKap (ORCPT ); Thu, 24 Mar 2011 06:30:45 -0400 In-Reply-To: <20110324102129.GF18867@8bytes.org> Sender: kvm-owner@vger.kernel.org List-ID: On 03/24/2011 12:21 PM, Joerg Roedel wrote: > On Thu, Mar 24, 2011 at 12:04:13PM +0200, Avi Kivity wrote: > > On 03/24/2011 09:40 AM, Joerg Roedel wrote: > >> This patch implements the propagation of the VM > >> virtual_tsc_khz into each vcpu data-structure to enable the > >> tsc-scaling feature. > >> static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset) > >> { > >> struct vcpu_svm *svm = to_svm(vcpu); > >> @@ -1093,6 +1123,9 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) > >> if (err) > >> goto free_svm; > >> > >> + if (!svm_vcpu_init_tsc(kvm, svm)) > >> + goto uninit; > >> + > > > > Need to set err. > > > > I'm not really happy about failing on vcpu creation. Prefer to fail > > when setting the frequency. > > Yes, in theory this shouldn't happen anyway because wrong frequencies > are catched when setting them via the ioctl. This is just another check, > but it should probably be turned into a BUG_ON in svm_vcpu_init_tsc() > because it is a bug that userspace could set an invalid khz value. Yes, I saw later. But BUG_ON() is too dangerous - if the checks don't exactly match a malicious user could cause a DOS. We can just avoid setting the scale and run with the host tsc frequency. -- error compiling committee.c: too many arguments to function