Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with "allow-emulation" option

[Alexander Graf <agraf@suse.de>]

On 05.06.14 19:48, Eduardo Habkost wrote:
> On Thu, Jun 05, 2014 at 06:58:17PM +0200, Alexander Graf wrote:
>> On 05.06.14 18:52, Paolo Bonzini wrote:
>>> Il 05/06/2014 18:45, Alexander Graf ha scritto:
>>>>> Only if you were using "-cpu somethingThatHasAVX", though, no?
>>>> Yes. The same argument goes the other way around. I want to use AVX
>>>> emulation, do "allow-emulation" and suddenly I get MONITOR/MWAIT
>>>> emulation.
>>> What about:
>>>
>>> - letting "-cpu foo,+emulatedfeature" just work
>>>
>>> - adding emulated=yes that blindly enables all emulated features
>>>
>>> - making "-cpu ...,check" prints a warning for emulated features
>>> unless emulated=yes
>> So:
>>
>>    -cpu foo,+emulatedFeature just works
> The problem with this is:
>
>   * -cpu foo,+emulatedFeature,enforce MUST fail. We don't want to enable
>     emulated/experimental features by accident, even if they appear in
>     the command-line expliclty.
>   * -cpu foo,+emulatedFeature (no "enforce" flag) MUST remove remove the
>     feature and report it on the "filtered-features" property, because
>     that's the only API available for management to check if a feature is
>     not available on GET_SUPPORTED_CPUID.
>
> That's why I suggest that the only way to enable emulatedFeature be
> using "allow-emulation=yes" explicitly on the command-line IN ADDITION
> to already having the feature included in the CPU model definition or in
> explicitly in the command-line.
>
> (or "allow-experimental-features", or whatever name we choose)
>
>>    -cpu foo,+notEmulatedFeature still sets the CPUID bit for that feature
> That's OK, but (I assume) you mean notEmulatedFeature is on
> GET_SUPPORTED_CPUID.
>
> Detailing the requirements:
>
>   * "-cpu foo,+SomeFeature" MUST NOT enable SomeFeature unless it is
>     present on GET_SUPPORTED_CPUID.
>   * "-cpu foo,+SomeFeature,enforce" MUST abort if the feature is not
>     present on GET_SUPPORTED_CPUID.
>   * "-cpu foo,+SomeFeature" MUST report SomeFeature on
>     "filtered-features", so management knows the feature is not set on
>     GET_SUPPORTED_CPUID.
>
> The items above are already part of the semantics of "-cpu" and if we
> change it, we risk defeating the whole purpose of GET_EMULATED_CPUID in
> the first place.
>
>>    -cpu foo,check prints warnings for all cpuid bits not in the
>> "allowed" bitmap. It prints different warnings depending on whether
>> the bit is in "emulated" or not
> That part makes sense. And we may also report GET_EMULATED_CPUID
> features on an "emulated-features" property, so management can get that
> information in a machine-friendly way.

I personally like the feature=force way of specifying forcefully enabled 
cpuid bits. Whether it's in GET_EMULATED_CPUID doesn't matter really. 
Only "check" should ever care about that bitmap.

But can we drop the EMULATED name somehow? Can we rename [1] the ioctl 
to say GET_UNSUPPORTED_CPUID or something along those lines? The name is 
just a really really bad pick.


Alex

[1] rename obviously means "introduce a new name with the same ioctl 
number and keep the old name around for legacy compatibility reasons"