Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On 06.06.14 04:37, Eduardo Habkost wrote: On Fri, Jun 06, 2014 at 03:21:04AM +0200, Borislav Petkov wrote: On Fri, Jun 06, 2014 at 12:24:26AM +0200, Alexander Graf wrote: 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. What do you mean, a bad pick :-P? I added extra care in naming that functionality what it is - bitfield in CPUID format of *emulated* features. Unsupported is wrong too - we do support them if we enable them explicitly. :-) How about GET_NOT_REALLY_FAST_BUT_STILL_NOT_FAST_ENOUGH_AS_IN_HW_FAST_CPUID? IMO, emulated on the kernel interface is good, because it describe what it is. Deciding which emulated features are experimental or good enough to be enabled implicitly even if emulated is policy for userspace to decide. allow-experimental is being mapped to GET_EMULATED_CPUID initially only because _by default_ the GET_EMULATED_CPUID-only features won't be enabled implicitly unless forced. But if one day we decide that emulation is good enough for a specific feature, we can make kvm_arch_get_supported_cpuid() return it even if it is present only on the GET_EMULATED_CPUID bitmap. Even if that decision depends on a specific implementation of that feature, the kernel can report that using KVM capabilities (to be checked by kvm_arch_get_supported_cpuid(), like we already do for tsc-deadline). Ok, works for me. I still don't see the point in having the bitmap at all then when we need to carry separate CAPs for individual features' working status, but if it makes people happy I'm ok with it. Alex
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Thu, Jun 05, 2014 at 06:26:41PM +0200, 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. Can you remind me what was your argument against KVM_GET_EMULATED_CPUID? Where do you want to move MOVBE to? Supported cpuid? That will make some CPU models unreasonably slow on hosts that do not support MOVBE natively. KVM is not in a busyness of instruction emulation, yes sometimes there is no choice (IO/real mode), sometimes emulating a feature is beneficial (x2apic) and sometimes guest cannot workaround missing feature, so by emulating it you allow guest functionality that is impossible otherwise (SVM/VMX). MOVBE (and MONITOR/MWAIT) is not in any of those categories. By forcing emulation upon unsuspecting guest you force it to use much slower alternative for what it can do by other means. However, for MONITOR/MWAIT it makes some sense. Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Gleb.
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Fri, Jun 06, 2014 at 01:16:00PM +0200, Alexander Graf wrote: On 06.06.14 04:37, Eduardo Habkost wrote: On Fri, Jun 06, 2014 at 03:21:04AM +0200, Borislav Petkov wrote: On Fri, Jun 06, 2014 at 12:24:26AM +0200, Alexander Graf wrote: 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. What do you mean, a bad pick :-P? I added extra care in naming that functionality what it is - bitfield in CPUID format of *emulated* features. Unsupported is wrong too - we do support them if we enable them explicitly. :-) How about GET_NOT_REALLY_FAST_BUT_STILL_NOT_FAST_ENOUGH_AS_IN_HW_FAST_CPUID? IMO, emulated on the kernel interface is good, because it describe what it is. Deciding which emulated features are experimental or good enough to be enabled implicitly even if emulated is policy for userspace to decide. allow-experimental is being mapped to GET_EMULATED_CPUID initially only because _by default_ the GET_EMULATED_CPUID-only features won't be enabled implicitly unless forced. But if one day we decide that emulation is good enough for a specific feature, we can make kvm_arch_get_supported_cpuid() return it even if it is present only on the GET_EMULATED_CPUID bitmap. Even if that decision depends on a specific implementation of that feature, the kernel can report that using KVM capabilities (to be checked by kvm_arch_get_supported_cpuid(), like we already do for tsc-deadline). Ok, works for me. I still don't see the point in having the bitmap at all then when we need to carry separate CAPs for individual features' working status, but if it makes people happy I'm ok with it. We won't necessarily need it, it is just an alternative we will always have. GET_*_CPUID is just an alternative capability system, which allow CPUID features to be automatically handled by generic QEMU code on many cases (instead of always requiring QEMU code changes for new features). But we can always get back to plain old KVM capabilities on more complex cases if necessary. I don't expect features to get promoted from GET_EMULATED_CPUID to GET_SUPPORTED_CPUID very often, because that's policy that can be handled by userspace (but that doesn't mean kernel developers can't do it if they think it is a good idea). -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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. 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. So could we add a new flag specifier instead? Today we have -flag and +flag. How about *flag to force enable if the kernel can handle it, but doesn't do so by default? Alex
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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. Paolo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
Il 05/06/2014 18:40, Alexander Graf ha scritto: 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. Only if you were using -cpu somethingThatHasAVX, though, no? Paolo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On 05.06.14 18:44, Paolo Bonzini wrote: Il 05/06/2014 18:40, Alexander Graf ha scritto: 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. 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. Alex
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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 Paolo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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 How about we remove the emulated=yes from this list? Then I'm happy :). Alex
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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 -cpu foo,+notEmulatedFeature still sets the CPUID bit for that feature -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 Alex
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
Il 05/06/2014 18:54, Alexander Graf ha scritto: 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 How about we remove the emulated=yes from this list? Then I'm happy :). So: - -cpu foo doesn't enable any emulated feature - -cpu foo,+movbe does - -cpu foo,check and -cpu foo,enforce print a nice and descriptive message if the feature is not available but could be enabled as emulated Ok? Paolo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Thu, Jun 05, 2014 at 06:57:57PM +0200, Paolo Bonzini wrote: Il 05/06/2014 18:54, Alexander Graf ha scritto: 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 How about we remove the emulated=yes from this list? Then I'm happy :). So: - -cpu foo doesn't enable any emulated feature What if foo already has movbe in the CPU model definition? - -cpu foo,+movbe does What if I want movbe enabled if and only if it is _not_ emulated? The whole point here is to never ever ever enable an emulated feature unless it was explicitly what the user wanted. - -cpu foo,check and -cpu foo,enforce print a nice and descriptive message if the feature is not available but could be enabled as emulated nice and descriptive message needs to be better specified. Messages on stderr are useless for management software. Maybe a emulated-features property in addition to the existing filtered-features would be useful. But any of the above changes the fact that this series does not change the semantics of any -cpu option combination, except when not using the enforce flag (which everybody who cares about CPUID data stability should be already using). -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
Il 05/06/2014 19:17, Eduardo Habkost ha scritto: If you don't want MONITOR/MWAIT you shouldn't be using a CPU model containing MONITOR/MWAIT in the first place. If you use -cpu somethingWithMONITOR, that means you are already asking QEMU for a CPU with MONITOR. If you were not getting MONITOR before using allow-emulation, that means you were not using the enforce flag, which you should be using, already. Except that many cpus don't work with enforce: $ /usr/libexec/qemu-kvm -cpu core2duo,enforce warning: host doesn't support requested feature: CPUID.01H:EDX.ds [bit 21] warning: host doesn't support requested feature: CPUID.01H:EDX.acpi [bit 22] warning: host doesn't support requested feature: CPUID.01H:EDX.ht [bit 28] warning: host doesn't support requested feature: CPUID.01H:EDX.tm [bit 29] warning: host doesn't support requested feature: CPUID.01H:EDX.pbe [bit 31] warning: host doesn't support requested feature: CPUID.01H:ECX.dtes64 [bit 2] warning: host doesn't support requested feature: CPUID.01H:ECX.monitor [bit 3] warning: host doesn't support requested feature: CPUID.01H:ECX.ds_cpl [bit 4] warning: host doesn't support requested feature: CPUID.01H:ECX.vmx [bit 5] warning: host doesn't support requested feature: CPUID.01H:ECX.est [bit 7] warning: host doesn't support requested feature: CPUID.01H:ECX.tm2 [bit 8] warning: host doesn't support requested feature: CPUID.01H:ECX.xtpr [bit 14] warning: host doesn't support requested feature: CPUID.01H:ECX.pdcm [bit 15] Like VMX above, some AMD models require nested SVM to be enabled: $ /usr/libexec/qemu-kvm -cpu phenom,enforce warning: host doesn't support requested feature: CPUID.01H:EDX.ht [bit 28] warning: host doesn't support requested feature: CPUID.01H:ECX.monitor [bit 3] warning: host doesn't support requested feature: CPUID.8001H:ECX.svm [bit 2] warning: host doesn't support requested feature: CPUID.800AH:EDX.npt [bit 0] warning: host doesn't support requested feature: CPUID.800AH:EDX.lbrv [bit 1] (actually, even with nested SVM, LBR virtualization is not supported in L2 guests) Testing some of the bits does not make sense, for example HT. Some others will never be supported (TM/TM2, ACPI, PBE, xTPR, ...). The set of CPUs that work with enforce is pretty much what you tested (the Opteron_G* models and some of the code-named Intel chips). Paolo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
Il 05/06/2014 19:19, Eduardo Habkost ha scritto: On Thu, Jun 05, 2014 at 06:57:57PM +0200, Paolo Bonzini wrote: Il 05/06/2014 18:54, Alexander Graf ha scritto: 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 How about we remove the emulated=yes from this list? Then I'm happy :). So: - -cpu foo doesn't enable any emulated feature What if foo already has movbe in the CPU model definition? It will be disabled. - -cpu foo,+movbe does What if I want movbe enabled if and only if it is _not_ emulated? Pick a CPU model that has it. The whole point here is to never ever ever enable an emulated feature unless it was explicitly what the user wanted. +foo could be enough. nice and descriptive message needs to be better specified. Messages on stderr are useless for management software. I'm not sure this feature is for management software users. Paolo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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. -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Thu, Jun 05, 2014 at 07:38:49PM +0200, Paolo Bonzini wrote: Il 05/06/2014 19:17, Eduardo Habkost ha scritto: If you don't want MONITOR/MWAIT you shouldn't be using a CPU model containing MONITOR/MWAIT in the first place. If you use -cpu somethingWithMONITOR, that means you are already asking QEMU for a CPU with MONITOR. If you were not getting MONITOR before using allow-emulation, that means you were not using the enforce flag, which you should be using, already. Except that many cpus don't work with enforce: $ /usr/libexec/qemu-kvm -cpu core2duo,enforce warning: host doesn't support requested feature: CPUID.01H:EDX.ds [bit 21] warning: host doesn't support requested feature: CPUID.01H:EDX.acpi [bit 22] warning: host doesn't support requested feature: CPUID.01H:EDX.ht [bit 28] warning: host doesn't support requested feature: CPUID.01H:EDX.tm [bit 29] warning: host doesn't support requested feature: CPUID.01H:EDX.pbe [bit 31] warning: host doesn't support requested feature: CPUID.01H:ECX.dtes64 [bit 2] warning: host doesn't support requested feature: CPUID.01H:ECX.monitor [bit 3] warning: host doesn't support requested feature: CPUID.01H:ECX.ds_cpl [bit 4] warning: host doesn't support requested feature: CPUID.01H:ECX.vmx [bit 5] warning: host doesn't support requested feature: CPUID.01H:ECX.est [bit 7] warning: host doesn't support requested feature: CPUID.01H:ECX.tm2 [bit 8] warning: host doesn't support requested feature: CPUID.01H:ECX.xtpr [bit 14] warning: host doesn't support requested feature: CPUID.01H:ECX.pdcm [bit 15] Probably this means the CPU model definition have AMD alises that shouldn't be there (as they are added automatically when CPU vendor is AMD). A bug in the CPU model. You still need enforce if you care about stable CPUID data. Like VMX above, some AMD models require nested SVM to be enabled: $ /usr/libexec/qemu-kvm -cpu phenom,enforce warning: host doesn't support requested feature: CPUID.01H:EDX.ht [bit 28] warning: host doesn't support requested feature: CPUID.01H:ECX.monitor [bit 3] warning: host doesn't support requested feature: CPUID.8001H:ECX.svm [bit 2] warning: host doesn't support requested feature: CPUID.800AH:EDX.npt [bit 0] warning: host doesn't support requested feature: CPUID.800AH:EDX.lbrv [bit 1] (actually, even with nested SVM, LBR virtualization is not supported in L2 guests) Testing some of the bits does not make sense, for example HT. Some others will never be supported (TM/TM2, ACPI, PBE, xTPR, ...). If a CPU model doesn't work with enforce, that means that _without_ enforce, you already risk getting CPUID data suddenly change on migration (if GET_SUPPORTED_CPUID changes). Isn't that exactly the problem Alex was complaining about? If existing CPU models have not very useful defaults that allow them to be useful with enforce, we may change them. But is not a reason to drop enforce, or you risk getting even worse problems. The set of CPUs that work with enforce is pretty much what you tested (the Opteron_G* models and some of the code-named Intel chips). So we need to fix this. Just like we fixed the MONITOR flag issue on KVM mode recently. -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Thu, Jun 05, 2014 at 06:45:16PM +0200, Alexander Graf wrote: On 05.06.14 18:44, Paolo Bonzini wrote: Il 05/06/2014 18:40, Alexander Graf ha scritto: 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. 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. This patch is just changing the set of _allowed_ bits, it is _not_ changing the actual semantics of a given -cpu option combination. Unless you are not using the enforce flag, but you should be using it if you care about not having CPUID data changing under your feet. If you don't want MONITOR/MWAIT you shouldn't be using a CPU model containing MONITOR/MWAIT in the first place. If you use -cpu somethingWithMONITOR, that means you are already asking QEMU for a CPU with MONITOR. If you were not getting MONITOR before using allow-emulation, that means you were not using the enforce flag, which you should be using, already. -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Thu, Jun 05, 2014 at 07:39:42PM +0200, Paolo Bonzini wrote: Il 05/06/2014 19:19, Eduardo Habkost ha scritto: On Thu, Jun 05, 2014 at 06:57:57PM +0200, Paolo Bonzini wrote: Il 05/06/2014 18:54, Alexander Graf ha scritto: 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 How about we remove the emulated=yes from this list? Then I'm happy :). So: - -cpu foo doesn't enable any emulated feature What if foo already has movbe in the CPU model definition? It will be disabled. I don't disagree with that. But not that if it will be disabled, that means -cpu foo,enforce will abort. But note that if you use -cpu foo without enforce, that means you can suddenly get movbe enabled once it gets included on GET_SUPPORTED_CPUID. So, if you care about predictable CPUID results and you want to enable an emulated/experimental feature, you have to do two things: 1) Make sure your setup works with enforce, so you know you will never get any feature suddenly and silently enabled. 2) Add +feature,allow-emulation=yes to the command-line, keep enforce and you will _not_ get any other experimental feature suddenly enabled (because now you are using enforce). - -cpu foo,+movbe does What if I want movbe enabled if and only if it is _not_ emulated? Pick a CPU model that has it. The whole point here is to never ever ever enable an emulated feature unless it was explicitly what the user wanted. +foo could be enough. We shouldn't do that, or we risk enabling features by accident and defeating the whole purpose of GET_EMULATED_CPUID. The current semantics of +foo is I want FOO, but only if it's on GET_SUPPORTED_CPUID. We shouldn't break it. nice and descriptive message needs to be better specified. Messages on stderr are useless for management software. I'm not sure this feature is for management software users. True. I am just worried about breaking current semantics that is used by management software. -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
Sorry for following the discussion backwards, but I see now that you started with a proposal that would cover both cases (the one you care about, and the one I care about), make both of us happy, but it was lost in favour of other suggestions I disagreed with: On Thu, Jun 05, 2014 at 06:24:22PM +0200, Alexander Graf wrote: 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. It is not an all-or-nothing approach if you use enforce, but now I see that you care about use cases of people that are _not_ using enforce (which I strongly recommend against, but we can try to cover those use cases, as well). 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. Agreed. Let's call it experimental. So could we add a new flag specifier instead? Today we have -flag and +flag. How about *flag to force enable if the kernel can handle it, but doesn't do so by default? Having an _additional_ way to allow emulation of specific flags, without changing the semantics of +flag would make both of us happy. But I believe we should look for a syntax that won't require special symbols, but just plain option names. Because the -cpu command-line options are simply translated to object_property_set() calls. +flag is legacy format, and we should move to use flag=on instead. So, what about, instead of *foo, having experimental-foo=on or force-foo=on? I was going to suggest foo=force, but that would require changing the data type of the properties from bool to string. -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
Sorry for replying to my own message, but I believe we can now summarize a possible solution that makes everybody happy, and the plans for it: On Thu, Jun 05, 2014 at 03:02:53PM -0300, Eduardo Habkost wrote: On Thu, Jun 05, 2014 at 07:39:42PM +0200, Paolo Bonzini wrote: Il 05/06/2014 19:19, Eduardo Habkost ha scritto: On Thu, Jun 05, 2014 at 06:57:57PM +0200, Paolo Bonzini wrote: Il 05/06/2014 18:54, Alexander Graf ha scritto: 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 How about we remove the emulated=yes from this list? Then I'm happy :). So: - -cpu foo doesn't enable any emulated feature What if foo already has movbe in the CPU model definition? It will be disabled. I don't disagree with that. But not that if it will be disabled, that means -cpu foo,enforce will abort. Typo. I meant note that. But note that if you use -cpu foo without enforce, that means you can suddenly get movbe enabled once it gets included on GET_SUPPORTED_CPUID. So, if you care about predictable CPUID results and you want to enable an emulated/experimental feature, you have to do two things: 1) Make sure your setup works with enforce, so you know you will never get any feature suddenly and silently enabled. 2) Add +feature,allow-emulation=yes to the command-line, keep enforce and you will _not_ get any other experimental feature suddenly enabled (because now you are using enforce). So, the above would cover the use cases I was thinking about. But I understand you have a different use case and you want to avoid GET_EMULATED_CPUID-related surprises even if not using enforce. For that, you need a more fine-tuned solution using *feature or feature=force. (Personally, I prefer feature=force instead of *feature). Implementing feature=force would be more cleanly implemented after we introduce the CPU feature properties, which we have been trying to include for 4 or 5 QEMU releases, and it was never merged. In the meantime, we could: * Include the less fine-tuned allow-emulation (or allow-experimental-features) option, which is implemented by this series, for people who use enforce and/or don't care too much about getting other experimental features enabled. * Wait until somebody implements feature=force. Personally, I don't care which plan we follow, as I am not an user of GET_EMULATED_CPUID. I will leave that decision for the QEMU maintainers and other developers. -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Thu, Jun 05, 2014 at 04:12:08PM -0300, Eduardo Habkost wrote: In the meantime, we could: * Include the less fine-tuned allow-emulation (or allow-experimental-features) option, which is implemented by this series, for people who use enforce and/or don't care too much about getting other experimental features enabled. * Wait until somebody implements feature=force. What are you going to do with allow-emulation after this? It will become an API and you'll have to support it. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. --
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On 06/05/2014 01:24 PM, Borislav Petkov wrote: On Thu, Jun 05, 2014 at 04:12:08PM -0300, Eduardo Habkost wrote: In the meantime, we could: * Include the less fine-tuned allow-emulation (or allow-experimental-features) option, which is implemented by this series, for people who use enforce and/or don't care too much about getting other experimental features enabled. * Wait until somebody implements feature=force. What are you going to do with allow-emulation after this? It will become an API and you'll have to support it. If you're worried about not needing the option forever, name it x-allow-emulation; we've already documented that anything with x- prefix is fair game for subsequent removal. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Thu, Jun 05, 2014 at 01:45:06PM -0600, Eric Blake wrote: On 06/05/2014 01:24 PM, Borislav Petkov wrote: On Thu, Jun 05, 2014 at 04:12:08PM -0300, Eduardo Habkost wrote: In the meantime, we could: * Include the less fine-tuned allow-emulation (or allow-experimental-features) option, which is implemented by this series, for people who use enforce and/or don't care too much about getting other experimental features enabled. * Wait until somebody implements feature=force. What are you going to do with allow-emulation after this? It will become an API and you'll have to support it. If you're worried about not needing the option forever, name it x-allow-emulation; we've already documented that anything with x- prefix is fair game for subsequent removal. Personally, I wouldn't mind about supporting it forever, as its implementation is very simple. But x-allow-emulation would be a good way to make GET_EMULATED_CPUID available for developers who want to test it, without any commitment to a specific API. -- Eduardo
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
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
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Fri, Jun 06, 2014 at 12:24:26AM +0200, Alexander Graf wrote: 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. What do you mean, a bad pick :-P? I added extra care in naming that functionality what it is - bitfield in CPUID format of *emulated* features. Unsupported is wrong too - we do support them if we enable them explicitly. :-) How about GET_NOT_REALLY_FAST_BUT_STILL_NOT_FAST_ENOUGH_AS_IN_HW_FAST_CPUID? :-P -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. --
Re: [Qemu-devel] [RFC 0/2] GET_EMULATED_CPUID support with allow-emulation option
On Fri, Jun 06, 2014 at 03:21:04AM +0200, Borislav Petkov wrote: On Fri, Jun 06, 2014 at 12:24:26AM +0200, Alexander Graf wrote: 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. What do you mean, a bad pick :-P? I added extra care in naming that functionality what it is - bitfield in CPUID format of *emulated* features. Unsupported is wrong too - we do support them if we enable them explicitly. :-) How about GET_NOT_REALLY_FAST_BUT_STILL_NOT_FAST_ENOUGH_AS_IN_HW_FAST_CPUID? IMO, emulated on the kernel interface is good, because it describe what it is. Deciding which emulated features are experimental or good enough to be enabled implicitly even if emulated is policy for userspace to decide. allow-experimental is being mapped to GET_EMULATED_CPUID initially only because _by default_ the GET_EMULATED_CPUID-only features won't be enabled implicitly unless forced. But if one day we decide that emulation is good enough for a specific feature, we can make kvm_arch_get_supported_cpuid() return it even if it is present only on the GET_EMULATED_CPUID bitmap. Even if that decision depends on a specific implementation of that feature, the kernel can report that using KVM capabilities (to be checked by kvm_arch_get_supported_cpuid(), like we already do for tsc-deadline). -- Eduardo
