From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 6/6] KVM: X86: Implement userspace interface to set	virtual_tsc_khz
Date: Thu, 24 Mar 2011 12:44:59 +0200
Message-ID: <4D8B20AB.6060101@redhat.com>
References: <1300952424-32014-1-git-send-email-joerg.roedel@amd.com> <1300952424-32014-7-git-send-email-joerg.roedel@amd.com> <4D8B198C.9020502@redhat.com> <20110324104106.GG18867@8bytes.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Joerg Roedel <joerg.roedel@amd.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Zachary Amsden <zamsden@redhat.com>, kvm@vger.kernel.org
To: Joerg Roedel <joro@8bytes.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:9147 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754560Ab1CXKpF (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 24 Mar 2011 06:45:05 -0400
In-Reply-To: <20110324104106.GG18867@8bytes.org>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 03/24/2011 12:41 PM, Joerg Roedel wrote:
> >>
> >>  +4.54 KVM_SET_TSC_KHZ
> >>  +
> >>  +Capability: KVM_CAP_TSC_CONTROL
> >>  +Architectures: x86
> >>  +Type: vm ioctl
> >>  +Parameters: __u32 (in)
> >>  +Returns: 0 on success, -1 on error
> >>  +
> >>  +Specifies the tsc frequency for the virtual machine. This IOCTL must be
> >>  +used before any vcpu is created. The unit of the frequency is KHz.
> >
> >  Should it not be a vcpu ioctl?
>
> The idea was that a vm ioctl will make sure that all vcpus in one vm
> have the same tsc frequency. With a vm ioctl we force this. So the tsc
> fequency is basically a vm capability which is mirrored into each vcpu
> data structure for performance reasons.
>

Yes - it doesn't make sense to have each vcpu run with a different tsc.  
But we do the same with cpuid, and since the tsc really is a per-cpu 
thing, then if it simplifies the code I think it's okay to make 
userspace call it per-cpu.

> >>  +
> >>  +		r = 0;
> >>  +		goto out;
> >>  +	}
> >>  +	case KVM_GET_TSC_KHZ: {
> >>  +		u32 vtsc_khz = kvm->arch.virtual_tsc_khz;
> >>  +
> >>  +		r = -EIO;
> >>  +		if (check_tsc_unstable())
> >>  +			goto out;
> >>  +
> >>  +		r = -EFAULT;
> >>  +		if (copy_to_user(argp,&vtsc_khz, sizeof(__u32)))
> >>  +			goto out;
> >
> >  And an ordinary return here.
>
> Okay, I'll change that. But I would prefer to keep this as a vm ioctl. A
> vcpu ioctl might be more flexible but I doubt anybody has a use-case for
> different tsc_khz values in one VM.

My motivation is simplification.

-- 
error compiling committee.c: too many arguments to function