Re: [PATCH 4/5] export new cpuid KVM_CAP

From: Avi Kivity <avi@redhat.com>
To: Glauber Costa <glommer@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] export new cpuid KVM_CAP
Date: Tue, 20 Apr 2010 12:29:05 +0300	[thread overview]
Message-ID: <4BCD73E1.3000207@redhat.com> (raw)
In-Reply-To: <20100419145037.GG14158@mothafucka.localdomain>

On 04/19/2010 05:50 PM, Glauber Costa wrote:
> On Sat, Apr 17, 2010 at 09:58:26PM +0300, Avi Kivity wrote:
>    
>> On 04/15/2010 09:37 PM, Glauber Costa wrote:
>>      
>>> Since we're changing the msrs kvmclock uses, we have to communicate
>>> that to the guest, through cpuid. We can add a new KVM_CAP to the
>>> hypervisor, and then patch userspace to recognize it.
>>>
>>> And if we ever add a new cpuid bit in the future, we have to do that again,
>>> which create some complexity and delay in feature adoption.
>>>
>>> Instead, what I'm proposing in this patch is a new capability, called
>>> KVM_CAP_X86_CPUID_FEATURE_LIST, that returns the current feature list
>>> currently supported by the hypervisor. If we ever want to add or remove
>>> some feature, we only need to tweak into the HV, leaving userspace untouched.
>>>
>>>        
>> Hm.  We need to update userspace anyway, since we don't like turning
>> features on unconditionally (it breaks live migration into an older
>> kernel).
>>      
> Right now, we don't have any mechanism to disable, say, kvmclock cpuid bit
> at userspace.

(that's a serious bug wrt migration, btw)

> But let's suppose we have: What's the difference between disabling
> it in the way it is now, and disabling it with the method I am proposing?
>    

No difference.

> All this ioctl say is: "Those are the current supported stuff in this HV".
> It does not mandate userspace to expose all of this to the guest. It just saves
> us from the job of creating yet another CAP for every bit we plan on including.
>    

Right.  Well, creating a new CAP and creating a new FEATURE flag aren't 
very different, and I'd like to avoid API churn.  We have enough new 
APIs due to missing or badly implemented features; I'd like to avoid new 
ones whenever possible.

It's not like it saves userspace anything, it has to accommodate older 
kernels anyhow.  We are able to ignore pre 2.6.27 kernels, but now with 
kvm shipped in long term support distributions, deprecating APIs will be 
much harder.

> If we want to be conservative, we can keep everything but the things we know
> already disabled, in userspace.
>    

We definitely need to do that.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.