[Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Nadav Har'El]

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(-)

--- .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 |

-- 
Nadav Har'El                        |      Tuesday, Jan  4 2011, 28 Tevet 5771
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |We are Microsoft. You will be
http://nadav.harel.org.il           |assimilated. Resistance is futile.

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Alexander Graf]

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

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Nadav Har'El]

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.

-- 
Nadav Har'El                        |      Tuesday, Jan  4 2011, 29 Tevet 5771
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |Sign above a shop selling burglar alarms:
http://nadav.harel.org.il           |"For the man who has everything"

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Alexander Graf]

-----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-----

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Nadav Har'El]

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.

-- 
Nadav Har'El                        |    Wednesday, Jan  5 2011, 29 Tevet 5771
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |Those who beat their swords into
http://nadav.harel.org.il           |plowshares will plow for those who don't.

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Avi Kivity]

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.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Nadav Har'El]

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.

-- 
Nadav Har'El                        |    Wednesday, Jan  5 2011, 29 Tevet 5771
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |I had a lovely evening. Unfortunately,
http://nadav.harel.org.il           |this wasn't it. - Groucho Marx

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Avi Kivity]

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.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Add VMX cpuid feature to qemu64

[Nadav Har'El]

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.

-- 
Nadav Har'El                        |    Wednesday, Jan  5 2011, 29 Tevet 5771
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |Seen on a sign outside a church: "This is
http://nadav.harel.org.il           |a C H _ _ C H ... what's missing?"