Re: [PATCH 6/6] KVM: X86: Implement userspace interface to set virtual_tsc_khz

[Avi Kivity <avi@redhat.com>]

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