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"

Reply via email to