| Message ID | 20110104150631.GA24238@fermat.math.technion.ac.il |
|---|---|
| State | New |
| Headers | show |
On 04.01.2011, at 16:06, Nadav Har'El wrote: > This patch adds the "VMX" cpuid feature to the default "qemu64" CPU type. > If KVM doesn't support this feature (i.e., nested VMX is not in the code, > or not enabled) it will mask out this bit. "qemu64" defines capabilities that qemu emulates. Qemu does not emulate VMX. So it doesn't belong there. I thought we had a "kvm64" type for your use case? > Note that other relevant CPU types, such as "core2duo" already correctly > include the VMX feature, and "qemu64" already includes the SVM feature "core2duo" really shouldn't include VMX as qemu simply can't emulate it. Qemu does emulate SVM, hence the bit in the "qemu64" type. Alex
On Tue, Jan 04, 2011, Alexander Graf wrote about "Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64": > > On 04.01.2011, at 16:06, Nadav Har'El wrote: > > > This patch adds the "VMX" cpuid feature to the default "qemu64" CPU type. > > If KVM doesn't support this feature (i.e., nested VMX is not in the code, > > or not enabled) it will mask out this bit. > > "qemu64" defines capabilities that qemu emulates. Qemu does not emulate VMX. So it doesn't belong there. I thought we had a "kvm64" type for your use case? I have to apologize for making such beginner's mistakes, because I am not very familiar with the inner workings of qemu. It seemed to me, but please correct me if I'm wrong, that when I use qemu-kvm, the default CPU type is "qemu64", not "kvm64", despite my using of KVM. In this is true, why shouldn't qemu64 indeed include all the bits that KVM may (perhaps) support? I wanted qemu-kvm to support nested VMX whenever the underlying KVM supports this, without needing to resort to special "-cpu ..." options. I'm not sure what you meant by Qemu emulating SVM as the reason why SVM (but not VMX) appears in qemu64's capabilities. As far as I understand, when you use KVM, qemu doesn't emulate any of the CPU capabilities - rather, any capability that KVM cannot support (such as nested SVM on an Intel processor) are removed from the CPUID capabilties list. Now that I think of it, perhaps I'm writing to the wrong mailing list? Is there a separate mailing list for the development of qemu-kvm, as opposed to the "general" qemu? I actually came here after being told by the KVM developers to write to the "qemu-devel" mailing list... > > Note that other relevant CPU types, such as "core2duo" already correctly > > include the VMX feature, and "qemu64" already includes the SVM feature > > "core2duo" really shouldn't include VMX as qemu simply can't emulate it. Qemu does emulate SVM, hence the bit in the "qemu64" type. Well, as you can see in target-i386/cpuid.c, at least in the qemu version I checked out from KVM's repository, the "core2duo" and "coreduo" cpu types do list the CPUID_EXT_VMX - and those are the only ones that list this feature. I don't understand why they should have this bit, and qemu64 not, or, alternatively - if none of those had the CPUID_EXT_VMX bit, how am I supposed to run qemu to allow KVM to do nested VMX? I can think of some tricks (the easiest is to use "-cpu host"), but why should these tricks be necessary when they aren't for nested SVM? And when real core2duo CPUs *do* have VMX, so if we try to accurately emulate them, we do need to support (nested) VMX? Thanks in advance, and again I apologize if I'm missing the obvious, or writing to the wrong mailing list.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04.01.2011, at 22:39, Nadav Har'El wrote: > On Tue, Jan 04, 2011, Alexander Graf wrote about "Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64": >> >> On 04.01.2011, at 16:06, Nadav Har'El wrote: >> >>> This patch adds the "VMX" cpuid feature to the default "qemu64" CPU type. >>> If KVM doesn't support this feature (i.e., nested VMX is not in the code, >>> or not enabled) it will mask out this bit. >> >> "qemu64" defines capabilities that qemu emulates. Qemu does not emulate VMX. So it doesn't belong there. I thought we had a "kvm64" type for your use case? > > I have to apologize for making such beginner's mistakes, because I am not > very familiar with the inner workings of qemu. > > It seemed to me, but please correct me if I'm wrong, that when I use qemu-kvm, > the default CPU type is "qemu64", not "kvm64", despite my using of KVM. > In this is true, why shouldn't qemu64 indeed include all the bits that KVM > may (perhaps) support? I wanted qemu-kvm to support nested VMX whenever the > underlying KVM supports this, without needing to resort to special "-cpu ..." > options. If qemu-kvm still uses the "qemu64" type, that's plainly a bug :). It really should use -cpu kvm64 / kvm32 as default. > I'm not sure what you meant by Qemu emulating SVM as the reason why SVM (but > not VMX) appears in qemu64's capabilities. As far as I understand, when you > use KVM, qemu doesn't emulate any of the CPU capabilities - rather, any > capability that KVM cannot support (such as nested SVM on an Intel processor) > are removed from the CPUID capabilties list. KVM is an optional accelerator. The cpu definitions are originally there for the TCG backend, so flags that TCG can't handle don't belong there. To work around that limitation, the kvm* cpu types were invented. You can run "qemu-system-x86_64 -cpu qemu64 -enable-kvm" which would disable the VMX bit if KVM masks it out. If however you start it with TCG using "qemu-system-x86_64 -cpu qemu64", the VMX bit would be set because TCG doesn't mask out anything, so you'd essentially break the emulation piece. > Now that I think of it, perhaps I'm writing to the wrong mailing list? > Is there a separate mailing list for the development of qemu-kvm, as > opposed to the "general" qemu? I actually came here after being told by > the KVM developers to write to the "qemu-devel" mailing list... The qemu-kvm and qemu trees are getting merged together. In the foreseeable future, there will only be qemu and qemu-kvm ceases to exist, because all additional functionality of it will be in upstream qemu. > >>> Note that other relevant CPU types, such as "core2duo" already correctly >>> include the VMX feature, and "qemu64" already includes the SVM feature >> >> "core2duo" really shouldn't include VMX as qemu simply can't emulate it. Qemu does emulate SVM, hence the bit in the "qemu64" type. > > Well, as you can see in target-i386/cpuid.c, at least in the qemu version > I checked out from KVM's repository, the "core2duo" and "coreduo" cpu types > do list the CPUID_EXT_VMX - and those are the only ones that list this feature. Oops, probably my bad :). > I don't understand why they should have this bit, and qemu64 not, or, > alternatively - if none of those had the CPUID_EXT_VMX bit, how am I supposed > to run qemu to allow KVM to do nested VMX? I can think of some tricks (the > easiest is to use "-cpu host"), but why should these tricks be necessary > when they aren't for nested SVM? And when real core2duo CPUs *do* have VMX, > so if we try to accurately emulate them, we do need to support (nested) VMX? If you add CPUID_EXT_VMX to the "kvm64" type and make that the default when kvm is enabled (like we did for nested svm), all should be well, no? > Thanks in advance, and again I apologize if I'm missing the obvious, or > writing to the wrong mailing list. You wrote to exactly the right mailing list, as this is a qemu issue :). Alex -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.16 (Darwin) iEYEARECAAYFAk0jlzwACgkQq7Wi27wfN1NpDACfdUAO/Y9zV6/ZAwinCSwPsSNR 05QAn3Zs9ESwBof1DeogBZB/pODlmGHS =/1lP -----END PGP SIGNATURE-----
On Tue, Jan 04, 2011, Alexander Graf wrote about "Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64": > If qemu-kvm still uses the "qemu64" type, that's plainly a bug :). It really should use -cpu kvm64 / kvm32 as default. When I run qemu-kvm's qemu-system-x86_64, and look at the CPU I get in the guest, I the CPU is clearly "QEMU Virtual CPU version 0.13.50" (qemu64) and not "Common KVM processor" (kvm64) the "-enable-kvm" option doesn't appear to change anything. Could this have always been a bug, which remained hidden in plain sight? :-) If I send a patch to default to kvm64, not qemu64, when KVM is being used, would that be acceptable by all parties involved (KVM people and QEMU people)? Also, kvm64 appears to have much fewer capabilities listed than qemu64. Was this list ever kept up to date with KVM's capabilities? On first glance it appars that even CPUID_EXT3_SVM is missing, so if kvm64 is used I think nested SVM will also stop working (in my patch, I can add this bit just like I plan to add CPUID_EXT_VMX). > > Well, as you can see in target-i386/cpuid.c, at least in the qemu version > > I checked out from KVM's repository, the "core2duo" and "coreduo" cpu types > > do list the CPUID_EXT_VMX - and those are the only ones that list this feature. > > Oops, probably my bad :). Do you want me to fix that in the patch as well? Thanks, Nadav. > If you add CPUID_EXT_VMX to the "kvm64" type and make that the default when kvm is enabled (like we did for nested svm), all should be well, no? I don't know, I don't know how the KVM users would react to their default CPU type being changed from qemu64 to kvm64. Perhaps Avi Kivity or somebody else has an opinion about this? Thanks, Nadav.
On 01/05/2011 10:17 AM, Nadav Har'El wrote: > On Tue, Jan 04, 2011, Alexander Graf wrote about "Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64": > > If qemu-kvm still uses the "qemu64" type, that's plainly a bug :). It really should use -cpu kvm64 / kvm32 as default. > > When I run qemu-kvm's qemu-system-x86_64, and look at the CPU I get in the > guest, I the CPU is clearly "QEMU Virtual CPU version 0.13.50" (qemu64) and > not "Common KVM processor" (kvm64) the "-enable-kvm" option doesn't appear to > change anything. Could this have always been a bug, which remained hidden > in plain sight? :-) > > If I send a patch to default to kvm64, not qemu64, when KVM is being used, > would that be acceptable by all parties involved (KVM people and QEMU people)? > The intent of kvm64/qemu64 is to provide some feature stability, so that when you run a guest with those cpu types, you get something expected. So adding to them isn't a good idea. I think we'd be fine with -cpu host and -cpu qemu64,+vmx enabling vmx support.
On Wed, Jan 05, 2011, Avi Kivity wrote about "Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64": > The intent of kvm64/qemu64 is to provide some feature stability, so that > when you run a guest with those cpu types, you get something expected. > So adding to them isn't a good idea. > > I think we'd be fine with -cpu host and -cpu qemu64,+vmx enabling vmx > support. Thanks. While this is ugly, I can certainly live with that (I've been telling people to use "-cpu host" to run nested VMX, until you asked me why I don't send a patch for qemu to fix that ;-)). Though I wonder why there shouldn't be some sort of "default" CPU type which just provides the maximum number of supported features, without attempting to provide stable features. If someone wants a specific stable CPU he can always use "-cpu core2duo" or even "-cpu qemu64", but shouldn't the default (when KVM is enabled) be to just provide all the features that KVM can provide? Anyway, what is your opinion on which should be the default in KVM - qemu64 or kvm64? If it's qemu64, does it make sense (as far as KVM is concerned) that it lists the SVM capability, but not the VMX capability? Thanks, Nadav.
On 01/05/2011 10:50 AM, Nadav Har'El wrote: > On Wed, Jan 05, 2011, Avi Kivity wrote about "Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64": > > The intent of kvm64/qemu64 is to provide some feature stability, so that > > when you run a guest with those cpu types, you get something expected. > > So adding to them isn't a good idea. > > > > I think we'd be fine with -cpu host and -cpu qemu64,+vmx enabling vmx > > support. > > Thanks. While this is ugly, I can certainly live with that (I've been telling > people to use "-cpu host" to run nested VMX, until you asked me why I don't > send a patch for qemu to fix that ;-)). Sorry about the confusion. > Though I wonder why there shouldn't be some sort of "default" CPU type which > just provides the maximum number of supported features, without attempting > to provide stable features. If someone wants a specific stable CPU he can > always use "-cpu core2duo" or even "-cpu qemu64", but shouldn't the default > (when KVM is enabled) be to just provide all the features that KVM can provide? That's -cpu host. People have been talking that it should be the default, and to a certain extent I agree. But changing defaults is dangerous business, it can break setups that rely on them. > Anyway, what is your opinion on which should be the default in KVM - qemu64 > or kvm64? If it's qemu64, does it make sense (as far as KVM is concerned) > that it lists the SVM capability, but not the VMX capability? IMO the emphasis on defaults is misplaced. Most people run virtual machines via a management tool, and we should focus on making it easy for management tool maintainers to do the right thing, by documenting our APIs and recommendations in the QEMU Management Tool Writer Guide.
On Wed, Jan 05, 2011, Avi Kivity wrote about "Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64": > IMO the emphasis on defaults is misplaced. Most people run virtual > machines via a management tool, and we should focus on making it easy > for management tool maintainers to do the right thing, by documenting > our APIs and recommendations in the QEMU Management Tool Writer Guide. Thanks. I'm withdrawing my patch, then, and will just keep the instruction to use "-cpu host" in Nested VMX's README. Just to add my last two cents on the subject, I think that from the same assumption you make ("management tool maintainers will do the right thing") I can draw an opposite conclusion: If qemu changes to use "kvm64" or "host" by default, the management tool maintainers will any do the right thing by explicitly stating the CPU type and not rely on the default, so nothing will break. At the same time, people who do use the command line will benefit, because they won't be baffled why the nested VMX feature, even if supported by the KVM module, is hidden from them, while nested SVM isn't. Thanks, Nadav.
--- .before/target-i386/cpuid.c 2011-01-04 17:00:21.000000000 +0200 +++ .after/target-i386/cpuid.c 2011-01-04 17:00:21.000000000 +0200 @@ -288,7 +288,8 @@ static x86_def_t builtin_x86_defs[] = { .features = PPRO_FEATURES | CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36, - .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT, + .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT | + CPUID_EXT_VMX, .ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) | CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
This patch adds the "VMX" cpuid feature to the default "qemu64" CPU type. If KVM doesn't support this feature (i.e., nested VMX is not in the code, or not enabled) it will mask out this bit. Note that other relevant CPU types, such as "core2duo" already correctly include the VMX feature, and "qemu64" already includes the SVM feature needed for nested SVM (again, KVM will remove this bit if it doesn't support nested SVM), so there is no reason not to list the VMX feature as well. Signed-off-by: Nadav Har'El <nyh@il.ibm.com> --- target-i386/cpuid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)