Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

Jan Kiszka wrote:
> On 2012-01-07 19:23, Liu, Jinsong wrote:
>> Jan Kiszka wrote:
>>> On 2012-01-05 18:07, Liu, Jinsong wrote:
>>>>> Sorry, it remains bogus to expose the tsc deadline timer feature
>>>>> on machines < pc-1.1. That's just like we introduced kvmclock
>>>>> only to pc-0.14 onward. The reason is that guest OSes so far
>>>>> running on qemu-1.0 or older without deadline timer support must
>>>>> not find that feature when being migrated to a host with qemu-1.1
>>>>> in pc-1.0 compat mode. Yes, the user can explicitly disable it,
>>>>> but that is not the idea of legacy machine models. They should
>>>>> provide the very same environment that older qemu versions
>>>>> offered. 
>>>>> 
>>>> 
>>>> Not quite clear about this point.
>>>> Per my understanding, if a kvm guest running on an older qemu
>>>> without tsc deadline timer support,
>>>> then after migrate, the guest would still cannot find tsc deadline
>>>> feature, no matter older or newer host/qemu/pc-xx it migrate to.
>>> 
>>> What should prevent this? The feature flags are not part of the
>>> vmstate. They are part of the vm configuration which is not migrated
>>> but defined by starting qemu on the target host.
>>> 
>> 
>> Thanks! understand this point ("They are part of the vm
>> configuration which is not migrated but defined by starting qemu on
>> the target host").  
>> 
>> But kvmclock example still cannot satisfy the purpose "guest running
>> on old qemu/pc-0.13 without kvmclock support must not find kvmclock
>> feature when being migrated to a host with new qemu/pc-0.13 compat
>>     mode". After migration, guest can possibily find kvmclock
>> feature CPUID.0x40000001.KVM_FEATURE_CLOCKSOURCE: pc_init1(...,
>> kvmclock_enabled) { pc_cpus_init(cpu_model);    // the point to
>> decide and expose cpuid features to guest   
>> 
>>     if (kvmclock_enabled) {        // the difference point between
>>     pc-0.13 vs. pc-0.14, related nothing to cpuid features.        
>> kvmclock_create(); } }
> 
> Right, not a perfect example: the cpuid feature is not influenced by
> this mechanism, only the fact if a kvmclock device (for save/restore)
> should be created. I guess we ignored this back then, only focusing on
> the more obvious issue of the addition device.
> 
>> 
>> Seems currently there is no good way to satisfy "guest running on
>> old qemu/pc-xx without feature A support must not find feature A
>> when being migrated to a host with new qemu/pc-xx compat mode", i.e.
>> considering   
>> * if running with '-cpu host' then migrate;
>> * each time we add a new cpuid feature it need add one or more new
>> machine model? is it necessary to bind pc-xx with cpuid feature? 
>> * logically cpuid features should better be controlled by cpu model,
>> not by machine model. 
> 
> The compatibility machines define the possible cpu models. If I select

How does machine define possible cpu models?
cpu model defined by qemu option '-cpu ...', while machine model defined by 
'-machine ...'

> pc-0.14, e.g. -cpu kvm64 should not give me features that 0.14 was not
> exposing.
> 

in such case, it's '-cpu kvm64' who take effect to decide what cpuid features 
would exposed to guest, not '-machine pc-0.14'.

IMO, what our patch need to do is to expose a cpuid feature to guest 
(CPUID.01H:ECX.TSC_Deadline[bit 24]), it decided by cpu model, not machine 
model:
pc_init1(..., cpu_model, ...)
{
    pc_cpus_init(cpu_model);   // this is the whole logic exposing cpuid 
features to guest
    ...
}

Do I misunderstanding something?

Thanks,
Jinsong