On Thu, Jun 05, 2014 at 06:40:25PM +0200, Alexander Graf wrote:
>
> 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);
Note that the above is not how it works today. It works this way:
requested_cpuid = cpu_model_cpuids(cpu_model);
requested_cpuid |= user_enabled_flags();
requested_cpuid &= ~user_disabled_flags();
possible_cpuid = cpuid & kvm_capable_cpuids();
if (possible_cpuid != requested_cpuid) {
if (check || enforce) fprintf(stderr, "some features are not available. help!");
if (enforce) exit(1);
cpu.filtered_features = cpuid & ~possible_cpuid;
}
cpu.cpuids = possible_cpuid;
This patch only affects the result of kvm_capable_cpuids(), only.
The semantics of a "-cpu" option is completely defined by
requested_cpuid. And this is not changing. The only way you can have a
different result depending on GET_SUPPORTED_CPUID or GET_EMULATE_CPUID,
is if you are _not_ using the "enforce" flag. But you should be using
it, if you care about stable CPUID data.
>
> 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.
If the user knows what he's doing, he simply sets allow-emulation, which
will _not_ affect requested_cpuid.
>
> 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.
If allow-emulation can suddenly enable a feature you don't want, your
command-line is already broken because changes to GET_SUPPORTED_CPUID
could also break your setup.
>
> 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.
Agreed. The main point of GET_EMULATED_CPUID is to report features we
will never want to enable accidentally. Calling them "experimental"
makes sense to me.
--
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
