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

[Alexander Graf <agraf@suse.de>]

On 05.06.14 18:26, Paolo Bonzini wrote:
> Il 05/06/2014 18:24, Alexander Graf ha scritto:
>>
>> On 05.06.14 18:12, Eduardo Habkost wrote:
>>> This implements GET_SUPPORTED_CPUID support using an explicit option
>>> for it:
>>> "allow-emulation". We don't want any emulated feature to be enabled by
>>> accident,
>>> so they will be enabled only if the user explicitly wants to allow 
>>> them.
>>
>> So is this an all-or-nothing approach? I would really prefer to override
>> individual bits.
>
> You can still disable them with "cpu foo,-movbe,allow-emulation".
>
>> Also, I don't think the line "emulated" is the right one to draw. We
>> "emulate" SVM or VMX too, but still enable them by default as soon as we
>> think they're ready enough.
>
> Well, I disagreed with the whole KVM_GET_EMULATED_CPUID concept for 
> MOVBE too for example.  It seemed overengineered to me, sooner or 
> later we might graduate MOVBE out of KVM_GET_EMULATED_CPUID as well.
>
> However, for MONITOR/MWAIT it makes some sense.

I honestly think what we want for MONITOR/MWAIT is a cpuid-override bit.

   cpuid = user_specified_cpuids();
   cpuid &= kvm_capable_cpuids()
   cpuid |= user_override_cpuids();

   kvm_set_cpuid(cpuid);

If the user knows what he's doing, he can set the force bit. If the 
kernel happens to emulate that instruction, he's happy. If the kernel 
doesn't emulate it, it will fail and he will realize that he did 
something stupid.

But ok, we do have this awesome GET_EMULATE_CPUID ioctl now, so we can 
as well use it - even though I consider it superfluous:

   cpuid = user_specified_cpuids();
   cpuid &= kvm_capable_cpuids()
   cpuid |= user_override_cpuids() & kvm_emulated_cpuid();

   kvm_set_cpuid(cpuid);

but enabling all experimental features inside KVM just because we want 
one or two of them is very counter-intuitive. Imagine we'd introduce 
emulation support for AVX. Suddenly allow-emulation (which I'd need for 
Mac OS X 10.5) would enable AVX as well which I really don't want enabled.

Also, while we can't change the ioctl name anymore, please let's use 
"experimental" rather than "emulated" as the name everywhere. Maybe 
we'll never bump individual features from experimental to fully 
supported, but there's no reason we wouldn't have emulated features that 
are not part of this bitmap and there's no reason we wouldn't have real 
hardware features that are not ready yet and part of this bitmap.


Alex