Re: [Qemu-devel] [PATCH] target-i386: add feature flags for CPUID[EAX=0xd, ECX=1]

[Paolo Bonzini]

On 26/11/2014 12:40, Eduardo Habkost wrote:
> On Wed, Nov 26, 2014 at 10:20:12AM +0100, Paolo Bonzini wrote:
>>
>>
>> On 25/11/2014 21:02, Paolo Bonzini wrote:
>>>>> +static const char *cpuid_xsave_feature_name[] = {
>>>>> +    "xsaveopt", "xsavec", "xgetbv1", "xsaves",
>>>>
>>>> None of the above features introduce any new state that might need to be
>>>> migrated, or will require other changes in QEMU to work, right?
>>>>
>>>> It looks like they don't introduce any extra state, but if they do, they
>>>> need to be added to unmigratable_flags until migration support is
>>>> implemented.
>>>>
>>>> If they require other QEMU changes, it would be nice if KVM reported
>>>> them using KVM_CHECK_EXTENSION instead of GET_SUPPORTED_CPUID, so it
>>>> wouldn't break "-cpu host".
>>>
>>> No, they don't.
>>
>> Actually, xsaves does but I don't think KVM_CHECK_EXTENSION is right.
>> It's just another MSR, and we haven't used KVM_CHECK_EXTENSION for new
>> MSRs and new XSAVE areas (last example: avx512).
> 
> If the changes needed are only to support migration (this is the case if
> it's just another MSR handled by KVM, or additional XSAVE areas),
> GET_SUPPORTED_CPUID is still reasonable, because features that are
> unknown to QEMU are always considered unmigratable until we add the
> feature name to the feature_name arrays. (That's why we need to know if
> the feature introduces additional state when adding the feature names to
> the array.)
> 
> If other changes are required to make the feature work even if no
> migration is required, then adding them to GET_SUPPORTED_CPUID would
> break "-cpu host" on older QEMUs. I don't think that's the case here,
> but I wanted to confirm.

KVM may need more changes (I don't know, the details of the feature are
not public yet), but a new userspace API is very unlikely based on Intel
documentation.

> (Should we add those observations to Documentation/virtual/kvm/api.txt?)
> 
>>
>> Since no hardware really exists for it, and KVM does not support it
>> anyway, I think it's simplest to leave xsaves out for now.  Is this right?
> 
> If unsure, it won't hurt to add the feature to unmigratable_features by
> now. Making QEMU aware of the feature name is still useful.

Ok, thanks.

Paolo